Bug 72239 – Accessibility: Chromium requires an AX notification when an iframe loads.

• Component: Accessibility
• Status: RESOLVED FIXED

[Dominic Mazzoni]

Chromium needs an AX notification when an iframe's document loads, because upon loading new renderers are created for the scroll area and document, invalidating the old ax objects associated with them, and Chromium needs to know to update its cache.

Sending an AXLayoutComplete will suffice for Chromium and will be harmless on Mac OS X.

[Dominic Mazzoni]

Created attachment 114882 [details]
Patch

[chris fleizach]

Comment on attachment 114882 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:2238
> +                    if (axIFrame)

my guess would be that since this is a new iframe, most of the time get() will return null because it won't be in the cache. you probably need to use getOrCreate

> LayoutTests/ChangeLog:9
> +

did this test already exist?

[Dominic Mazzoni]

Comment on attachment 114882 [details]
Patch

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

>> Source/WebCore/dom/Document.cpp:2238
>> +                    if (axIFrame)
> 
> my guess would be that since this is a new iframe, most of the time get() will return null because it won't be in the cache. you probably need to use getOrCreate

Actually I used get() on purpose because it's not necessary to send the notification if it's not in the cache yet.

Is that okay or do you think it'd be preferable to always create the AccessibleObject?

[chris fleizach]

(In reply to comment #3)
> (From update of attachment 114882 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=114882&action=review
> 
> >> Source/WebCore/dom/Document.cpp:2238
> >> +                    if (axIFrame)
> > 
> > my guess would be that since this is a new iframe, most of the time get() will return null because it won't be in the cache. you probably need to use getOrCreate
> 
> Actually I used get() on purpose because it's not necessary to send the notification if it's not in the cache yet.
> 
> Is that okay or do you think it'd be preferable to always create the AccessibleObject?

I think we should do getOrCreate, otherwise we'll have to change it later when we realize we need to monitor these changes (and that might be the screen reader itself that wants to monitor it)... the topDocument() case comment says that it needs to make the object first, so it should probably happen for sub frames as well

[Dominic Mazzoni]

Sounds good, I'm updating it to use getOrCreate.

Also I apparently neglected to include the new test before. Adding it this time, it should make more sense now.

[Dominic Mazzoni]

Created attachment 115528 [details]
Patch

[chris fleizach]

Comment on attachment 115528 [details]
Patch

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

r=me

> Source/WebCore/dom/Document.cpp:2232
> +            // LayoutComplete notification on it.

comment should explain why we need to post this notification (not what it is doing [which is evident from the code])

[Dominic Mazzoni]

Created attachment 115530 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 115530 [details]
Patch for landing

Clearing flags on attachment: 115530

Committed r100584: <http://trac.webkit.org/changeset/100584>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Peter Kasting]

Reverted r100698 for reason:

This change wasn't the problem either.

Committed r100705: <http://trac.webkit.org/changeset/100705>

[Peter Kasting]

webkit-patch should not have reopened.