Bug 137382

Summary: AX: Going back is broken for VoiceOver
Product: WebKit Reporter: chris fleizach <cfleizach>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, buildbot, commit-queue, dmazzoni, esprehn+autocc, jcraig, jdiggs, kangil.han, mario, rniwa, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
New patch
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
none
patch darin: review+

Description chris fleizach 2014-10-02 23:29:13 PDT
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>
Comment 1 chris fleizach 2014-10-02 23:32:11 PDT
Created attachment 239181 [details]
patch
Comment 2 Mario Sanchez Prada 2014-10-05 10:29:50 PDT
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?
Comment 3 chris fleizach 2014-10-05 15:43:49 PDT
(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
Comment 4 James Craig 2014-10-06 12:14:15 PDT
Repro is more consistent if you select "View > Show Tab Bar"
Comment 5 chris fleizach 2014-10-13 17:22:04 PDT
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
Comment 6 Build Bot 2014-10-13 18:58:04 PDT
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
Comment 7 Build Bot 2014-10-13 18:58:08 PDT
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
Comment 8 Build Bot 2014-10-13 19:08:05 PDT
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
Comment 9 Build Bot 2014-10-13 19:08:14 PDT
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
Comment 10 chris fleizach 2014-10-14 22:42:19 PDT
Created attachment 239850 [details]
patch
Comment 11 chris fleizach 2014-10-15 12:35:43 PDT
http://trac.webkit.org/changeset/174741