Bug 137382 – AX: Going back is broken for VoiceOver

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

[chris fleizach]

Using VoiceOver, do the following, and you won't be able to interact with the HTML content at all:

1) Open Safari with VoiceOver enabled.
2) Command-l, search for apple wikipedia.
3) Use VO-right arrow to navigate to the HTML content if not already interacting, press VO-Shift-down arrow to interact.
4) Press VO-Command-h to move through headings until finding the first search result. It will be Apple's wikipedia page.
5) Press VO-space to activate the link. The header only contains a single link, so this is expected.
6) Use VO-Command-h to move through headings, move through several sections. Use VO-right and left to explore the plain text of one of the sections.
7) Press COmmand-[ (left bracket) to go back a page.
8) If still interacting with HTML, try VO-left and right.
9) If not, or if nothing works, stop interacting with VO-Shift-up and then VO-Shift-down to try interacting again.

<rdar://problem/18199992>

[chris fleizach]

Created attachment 239181 [details]
patch

[Mario Sanchez Prada]

Comment on attachment 239181 [details]
patch

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

> Source/WebCore/dom/Document.cpp:2407
> +    if (loadingFinished && !doload)
> +        processAccessibilityLoadNotification();

Would it work if you just do "if (loadingFinished) processAccessibilityLoadNotification();", and remove the call to processAccessibilityLoadNotification() below?

[chris fleizach]

(In reply to comment #2)
> (From update of attachment 239181 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239181&action=review
> 
> > Source/WebCore/dom/Document.cpp:2407
> > +    if (loadingFinished && !doload)
> > +        processAccessibilityLoadNotification();
> 
> Would it work if you just do "if (loadingFinished) processAccessibilityLoadNotification();", and remove the call to processAccessibilityLoadNotification() below?

I think it might. I need to double check first. Thanks

[James Craig]

Repro is more consistent if you select "View > Show Tab Bar"

[chris fleizach]

Created attachment 239762 [details]
New patch

Further investigation uncovered more serious problems

1) I was still missing load notifications for some subset or pages in the history cache. Luckily there's already an existing mechanism that tracks frame loads

2) The scroll view sometimes cached its children too long. So we should get rid of that caching. I think there's little performance penalty to putting the same 3 elements back in, and we won't have to worry about caching the wrong elements

[Build Bot]

Comment on attachment 239762 [details]
New patch

Attachment 239762 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5945514394648576

New failing tests:
accessibility/loading-iframe-sends-notification.html

[Build Bot]

Created attachment 239770 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 239762 [details]
New patch

Attachment 239762 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4851444490436608

New failing tests:
accessibility/loading-iframe-sends-notification.html

[Build Bot]

Created attachment 239773 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[chris fleizach]

Created attachment 239850 [details]
patch

[chris fleizach]

http://trac.webkit.org/changeset/174741