Bug 72382 – [Gtk] atk_text_get_text_at_offset() sometimes fails to provide the correct line

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Joanmarie Diggs]

Created attachment 115151 [details]
test script

Steps to reproduce:

1. View http://www.vogella.de/articles/OSGi/article.html in epiphany
2. Launch the attached test script
3. In epiphany, press F7 to enable caret browsing
4. Position the caret at the heading '3.2 Coding'
5. Begin Down Arrowing

Results: The lines of text are correctly reported outside of the code examples. Inside the examples, characters are reported for lines on which they do not appear, and all lines seem to be incorrect.

(Perhaps a failed/overlooked 'flattening' case??)

[Mario Sanchez Prada]

Seems to be related to an additional '\n' that is being manually added in some cases, at the end of a line. See https://bugs.webkit.org/show_bug.cgi?id=25415#c78.

Now working on it...

[Mario Sanchez Prada]

Created attachment 122263 [details]
Patch proposal + Unit test

[Mario Sanchez Prada]

Comment on attachment 122263 [details]
Patch proposal + Unit test

View in context: https://bugs.webkit.org/attachment.cgi?id=122263&action=review

> Source/WebKit/gtk/tests/testatk.c:796
> +static void testWebkitAtkGetTextAtOffserWithPreformattedText()

Obviously, this is a typo. The right name would be testWebkitAtkGetTextAtOffsetWithPreformattedText()

[Mario Sanchez Prada]

Created attachment 122750 [details]
Patch proposal + Unit test

New patch, fixing some stupid mistakes in the previous one.

[Mario Sanchez Prada]

Created attachment 123799 [details]
Patch proposal + Unit Test

Updating the patch after the refactor done for fixing bug 76783

[Martin Robinson]

Comment on attachment 123799 [details]
Patch proposal + Unit Test

View in context: https://bugs.webkit.org/attachment.cgi?id=123799&action=review

> Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:-105
> -            box = box->nextTextBox();

Did you mean to remove this line? I don't see anyting about this in the ChangeLog?

[Mario Sanchez Prada]

Comment on attachment 123799 [details]
Patch proposal + Unit Test

While smoke testing some stuff with MiniBrowser and Orca, I found out today that this patch seems to cause some hangs when browsing with the caret, so I'm withdrawing the r? flag for now.

[Joanmarie Diggs]

(In reply to comment #7)
> (From update of attachment 123799 [details])
> While smoke testing some stuff with MiniBrowser and Orca, I found out today that this patch seems to cause some hangs when browsing with the caret

Found an Orca-free way to reliably reproduce the hang:

1. Launch the tester (below)
2. Launch Epiphany
3. Navigate via the caret

I get reliable and immediate hangs in Epiphany, along with the following pyatspi lookup error:

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/pyatspi/registry.py", line 193, in eventWrapper
    return callback(event)
  File "./caret-moved.py", line 11, in listener
    print text.getTextAtOffset(event.detail1, pyatspi.TEXT_BOUNDARY_LINE_START)
  File "/usr/lib/python2.7/site-packages/pyatspi/text.py", line 443, in getTextAtOffset
    ret = exwrap(Atspi.Text.get_text_at_offset, self.obj, offset, type)
  File "/usr/lib/python2.7/site-packages/pyatspi/utils.py", line 339, in exwrap
    raise LookupError
LookupError

BTW pyatspi LookupErrors are the spawn of <insert your personal devil here>. I'll poke Mike Gorse.

--------------------
#!/usr/bin/python

import pyatspi

def listener(event):
    try:
        text = event.source.queryText()
    except NotImplementedError:
        return

    print text.getTextAtOffset(event.detail1, pyatspi.TEXT_BOUNDARY_LINE_START)

pyatspi.Registry.registerEventListener(listener, "object:text-caret-moved")
pyatspi.Registry.start()
--------------------

[Joanmarie Diggs]

(In reply to comment #6)
> (From update of attachment 123799 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123799&action=review
> 
> > Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:-105
> > -            box = box->nextTextBox();
> 
> Did you mean to remove this line? I don't see anyting about this in the ChangeLog?

So... I'm going to test having put that line back. Unfortunately, I'm still building (building building) WebKitGtk due to having built Gtk+ and Glib from master. But in the meantime, it sure looks to me that this may be the source of the hang because I don't see anything else updating the value of box which is being looped over.

[Joanmarie Diggs]

So finally the build completed. I did some quick testing of WebKit from master with the patch proposed with the accidentally removed line put back. Bug seems to be fixed, hang is definitely gone.

[Mario Sanchez Prada]

Created attachment 124990 [details]
Patch proposal + Unit Test

[Mario Sanchez Prada]

(In reply to comment #6)
> (From update of attachment 123799 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123799&action=review
> 
> > Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:-105
> > -            box = box->nextTextBox();
> 
> Did you mean to remove this line? I don't see anyting about this in the ChangeLog?

(In reply to comment #10)
> So finally the build completed. I did some quick testing of WebKit from master with the patch proposed with the accidentally removed line put back. Bug seems to be fixed, hang is definitely gone.

You both were right. Sorry for this mistake, and for making you waste your time... now attached the right one

[Mario Sanchez Prada]

Committed r106545: <http://trac.webkit.org/changeset/106545>