Bug 72382

Summary: [Gtk] atk_text_get_text_at_offset() sometimes fails to provide the correct line
Product: WebKit Reporter: Joanmarie Diggs (irc: joanie) <jdiggs@igalia.com>
Component: AccessibilityAssignee: Mario Sanchez Prada <mario@webkit.org>
Status: RESOLVED FIXED    
Severity: Normal CC: mario@webkit.org, mrobinson@webkit.org, pnormand@igalia.com
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 25531, 73431    
Attachments:
Description Flags
test script
none
Patch proposal + Unit test
none
Patch proposal + Unit test
none
Patch proposal + Unit Test
none
Patch proposal + Unit Test mrobinson: review+

Description From 2011-11-15 05:58:47 PST
Created an attachment (id=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??)
------- Comment #1 From 2012-01-12 07:49:22 PST -------
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...
------- Comment #2 From 2012-01-12 09:49:33 PST -------
Created an attachment (id=122263) [details]
Patch proposal + Unit test
------- Comment #3 From 2012-01-17 00:50:42 PST -------
(From update of attachment 122263 [details])
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()
------- Comment #4 From 2012-01-17 04:59:48 PST -------
Created an attachment (id=122750) [details]
Patch proposal + Unit test

New patch, fixing some stupid mistakes in the previous one.
------- Comment #5 From 2012-01-24 13:52:30 PST -------
Created an attachment (id=123799) [details]
Patch proposal + Unit Test

Updating the patch after the refactor done for fixing bug 76783
------- Comment #6 From 2012-01-24 13:59:28 PST -------
(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?
------- Comment #7 From 2012-01-30 14:31:38 PST -------
(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, so I'm withdrawing the r? flag for now.
------- Comment #8 From 2012-02-01 07:38:56 PST -------
(In reply to comment #7)
> (From update of attachment 123799 [details] [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()
--------------------
------- Comment #9 From 2012-02-01 09:30:50 PST -------
(In reply to comment #6)
> (From update of attachment 123799 [details] [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.
------- Comment #10 From 2012-02-01 10:58:19 PST -------
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.
------- Comment #11 From 2012-02-01 12:30:28 PST -------
Created an attachment (id=124990) [details]
Patch proposal + Unit Test
------- Comment #12 From 2012-02-01 12:31:44 PST -------
(In reply to comment #6)
> (From update of attachment 123799 [details] [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
------- Comment #13 From 2012-02-02 05:18:26 PST -------
Committed r106545: <http://trac.webkit.org/changeset/106545>