After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 383000 - Some visible text skipped when reviewing in Firefox
Some visible text skipped when reviewing in Firefox
Status: RESOLVED FIXED
Product: lsr
Classification: Deprecated
Component: core
0.3.x
Other Linux
: Urgent normal
: 0.4.0
Assigned To: Scott Haeger
LSR maintainers
Depends on:
Blocks:
 
 
Reported: 2006-12-06 13:29 UTC by Peter Parente
Modified: 2007-01-15 14:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test webpage that illustrates problem (482 bytes, text/plain)
2006-12-20 22:18 UTC, Scott Haeger
  Details
fix for clipping textboxes when only_visible is selected (463 bytes, patch)
2006-12-21 20:41 UTC, Scott Haeger
committed Details | Review
files to demonstrate <p> cropping bug (3.10 KB, application/x-compressed-tar)
2006-12-22 15:07 UTC, Scott Haeger
  Details
patch for Some visible text skipped when reviewing in Firefox (2.05 KB, patch)
2007-01-11 21:38 UTC, Scott Haeger
rejected Details | Review

Description Peter Parente 2006-12-06 13:29:35 UTC
Some text gets skipped when using the review keys in Firefox. This especially happens when reading a paragraph with other objects embedded in it. Bug is likely in AccessibleItemWalker or in HypertextAdapter. If not, bug could be in the way FF3 is reporting next/prev item.
Comment 1 Scott Haeger 2006-12-20 22:18:12 UTC
Created attachment 78720 [details]
test webpage that illustrates problem
Comment 2 Scott Haeger 2006-12-20 22:38:51 UTC
The test webpage (see attachments) demonstrates the problem.  Here are some observations:

 1. The paragraph (<p>) is offset by an acc.item_offset number of characters.  
 2. The bug occurs when the <div> is included before the <p>.
 3. The bug does not occur if <p> is replaced by <span>.
 4. The bug is most likely in HypertextAdapter.getNextItem().  <span>s filter through the last else while <p> filter through the second if.
 5.  Before HypertextAdapter.getNextItem() the por item_offset is 0, after the method the item_offset is 127.
Comment 3 Peter Parente 2006-12-21 00:00:17 UTC
The use of item_offset in hypertext mimicks text controls in a sense. The item_offset is the starting character for a "chunk" of text. In this case, the "chunk" is some child accessible indicated by the unicode embed character (0xfffe). So what you're seeing in #5 might be the correct behavior for the next POR. The first chunk of hypertext spans characters 0 through 126. Then there's probably an embed (e.g. a link) at character 127. Following that there might be more text, so the next POR might be POR(hypertext acc, 128, 0).

Keep in mind whether review visible items is on or off during your tests. You'll get different results in each case. This bug is about making the review keys behavior properly when reviewing visible items only.
Comment 4 Peter Parente 2006-12-21 16:21:29 UTC
> I saw your recent comments about 383000.  Take a look at the test web
> page I uploaded.  There aren't any hyperlinks, so I don't understand how
> the embed character is even there.

Please post the text as shown by at-poke for each of the nodes in the accessible hierarchy under the document frame? I don't have convenient access to FF3 right now.

> Another observation is that the "document frame" acc passes
> HypertextAdapter.when().  This doesn't seem right.

The document frame almost always has embed characters representing the top level elements, which means it counts as hypertext. In your case, I bet it has two: one for the div and the other paragraph. Please post the node text and we'll confirm.
Comment 5 Scott Haeger 2006-12-21 17:27:57 UTC
The document frame has two embed characters as you mentioned.  The following is text from the elements below the document frame.  Nothing special there.

section =  test line 
paragraph = This is a test line for bug 383000.  This is the second sentence.  This is averylongwordthe third sentence. This is the fourth sentence.  This is the fifth sentence. This is the sixth sentence.  This is the seventh sentence.  This is the eighth sentence.  This is the ninth sentence.  This is the tenth sentence. 

The paragraph is being handled properly when "view visible only" is unchecked.  However, when checked, TextAdapter._getVisibleItemExtents() returns bad first and last values.  Shouldn't text within a paragraph that is clearly visible on the screen be considered "visible"?
Comment 6 Scott Haeger 2006-12-21 18:02:12 UTC
New TextAdapter method.  FUDGE factors were not implemented correctly resulting in the text box being clipped improperly.  Patch coming soon.


  def _getVisibleItemExtents(self, only_visible):
    acc = self.accessible
    comp = Interfaces.IComponent(acc)
    text = Interfaces.IText(acc)
    if only_visible:
      e = comp.getExtents(Constants.WINDOW_COORDS)
      # get the first item
      x, y = e.x-FUDGE_PX, e.y-FUDGE_PX
      print "TextAdapter _getVisibleItemExtents first coords=", x,y
      first = text.getOffsetAtPoint(x, y, Constants.WINDOW_COORDS)
      # get the last item
      x, y = e.x+e.width+FUDGE_PX, e.y+e.height+FUDGE_PX
      print "TextAdapter _getVisibleItemExtents last coords=", x,y
      last = text.getOffsetAtPoint(x, y, Constants.WINDOW_COORDS)
      print "TextAdapter _getVisibleItemExtents if"
    else:
      first = 0
      last = text.characterCount-1
      print "TextAdapter _getVisibleItemExtents else"
    if first == last:
      # nit: protection against bad bounds reporting
      first = 0
      last = text.characterCount-1
    print "TextAdapter _getVisibleItemExtents returning", first, last
    return first, last
Comment 7 Peter Parente 2006-12-21 18:04:54 UTC
    > Shouldn't text within a paragraph that is clearly visible on
    the screen be considered "visible"?

    Yep. This sounds like a Firefox bug. Create an account of Mozilla's Bugzilla
    page. Then open a new FF bug explaining the problem using this link:

    https://bugzilla.mozilla.org/enter_bug.cgi?product=Core&version=Trunk&component=Disability%20Access%20APIs&rep_platform=PC&op_sys=Linux&priority=--&bug_severity=normal&target_milestone=---&bug_status=NEW&assigned_to=aaronleventhal%40moonset.net&qa_contact=accessibility-apis%40core.bugs&cc=&bug_file_loc=http%3A%2F%2F&short_desc=&comment=&commentprivacy=0&keywords=&dependson=&blocked=&maketemplate=Remember%20values%20as%20bookmarkable%20template&form_name=enter_bug

    It should fill in the component information properly. Be sure to include the
    nightly build info in the bug report (see Help -> About, copy & paste the
    version, build, and date info from the text box).

Comment 8 Scott Haeger 2006-12-21 20:41:18 UTC
Created attachment 78752 [details] [review]
fix for clipping textboxes when only_visible is selected

This patch fixes TextAdapter._getVisibleItemExtents().  The fudge factor applied to getExtents() use to create a clipping of the textbox resulting in missing lines of text.  The fudge factor now allows getOffsetAtPoint() to "see" the entire textbox.  Hopefully, this change does not affect browsing of textboxes in other areas.  The question arises,  Why have a fudge factor?
Comment 9 Peter Parente 2006-12-21 21:40:33 UTC
The fudge factor was added to account for list, table, and tree controls that have a border. If use the getAccessibleAtPoint call with a coordinate that falls on the border, the return value was None in some cases. We added a fudge factor of a few pixels to move in from the border so the call returns one of the list items.

This may or may not be a problem anymore. AT-SPI might have been updated to account for visual borders. The border problem may also not exist for text boxes.

Test this patch in a few multiline text boxes. gedit and gnome-terminal are good test cases. Report on the results here. I'll apply the patch if it works properly.
Comment 10 Scott Haeger 2006-12-22 15:07:20 UTC
Created attachment 78793 [details]
files to demonstrate <p> cropping bug
Comment 11 Scott Haeger 2006-12-22 15:07:50 UTC
Test results.  See attached html test files.

                 Before patch                          After patch
HTML tests
test1            pass                                  pass
test2            failed; cropped <p>                   pass
test3            failed; cropped <p>                   pass
test4            pass                                  pass
test5            pass                                  pass
test6            failed; cropped <p>                   pass                             

GTK-Demo
All text         pass                                  pass
Tree store       pass                                  pass
List             pass                                  pass
Gedit            pass                                  pass
GNOME Term       pass                                  pass
Comment 12 Peter Parente 2006-12-22 18:46:32 UTC
Excellent work, Scott. Committing the patch.
Comment 13 Peter Parente 2007-01-10 14:39:45 UTC
Switching the fudge factors from +/- may have fixed the problem, but I'm not sure if it was just a side effect or the true solution. When looking for the first character, the fudge factor should be added, not subtracted, because the 0,0 coordinate is in the top left corner of the screen. For the last character, it should be subtracted, not added, because xmax, ymax is in the lower right.

I'm leaving this patch in place, but I need an explanation as to why it's working. I'm seeing weird behavior when reviewing visible items in gnome-terminal now, and I wonder if it's a result of this change or some other change in the code.

Plus, I suspect there are still problems with how getOffsetAtPoint is working in Firefox. This patch may have worked around those bugs, but I think the real problem lies deeper. See the comments in bug #382998.
Comment 14 Peter Parente 2007-01-10 18:55:18 UTC
Questions from Scott:

> Does the GTerminal weirdness occur when the change is backed out?

Yes. The terminal weirdness was the result of another bug.

> Can you give me an example of the problem in GTerminal?  Is it simply review key errors?

It had to do with continuous reading which I now fixed.

> I will test with no fudge factor.  Was the origin of the fudge factor related to problems in the tree and list type elements?

Unknown. I'm unclear as to how it works either way in text fields. Given the x,y coordinate one of 1) adding a fudge factor or 2) subtracting a fudge factor should work while the other should not. What's weird is that they both appear to be working.

The current TextAdapter arithmetic (the one specified in the last patch on this bug) is contrary to all other adapters that add/subtract fudge factors. Some experimentation using at-poke also seems to suggest that the new patch is adding when it should be subtracting and vice versa. For instance, scrolling a web page down decreases the y coordinate indicating that y=0 is at the top of the screen. Therefore, when testing for the first character in a rectangular region, we want to move down a few pixels to find it.
Comment 15 Scott Haeger 2007-01-11 21:38:56 UTC
Created attachment 80074 [details] [review]
patch for Some visible text skipped when reviewing in Firefox

Firefox Component::getExtents() returns screen coordinates when requesting both screen and window coordinates.  Text::getOffsetAtPoint() now uses screen coordinates as a work around to the Firefox bug.
Comment 16 Scott Haeger 2007-01-11 21:55:41 UTC
Additional information

text.getOffsetAtPoint() may return -1 if the point is outside the bounds.  This is contrary to what has been seen in gedit where 0 or max is returned.
Comment 17 Peter Parente 2007-01-15 14:20:18 UTC
Patch rejected in favor of overlapping changes in patch for bug #395584.

Fixed in the development version. The fix will be available in the next major release. Thank you for your bug report.