Bug 86317

Summary: Seamless iframe should not announce a new browsing context
Product: WebKit Reporter: Lars Gunther <webmaster>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Severity: Normal CC: abarth, apinheiro, cfleizach, dglazkov, dmazzoni, eric, fishd, jamesr, jdiggs, peter+ews, tkent+wkapi, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 45950    
Description Flags
eric: review+, webkit.review.bot: commit-queue-
buildbot: commit-queue-
patch for landing none

Description Lars Gunther 2012-05-13 06:59:16 PDT
According to the spec, a seamless iframe should not be announced as a new browsing context to AT users, whereas a regular iframe should.

See also comment 12 and 13 in bug 45950 https://bugs.webkit.org/show_bug.cgi?id=45950#c12
Comment 1 Eric Seidel (no email) 2012-05-14 15:41:25 PDT
I've confirmed that voiceover announces "frame" for a seamless iframe, unlike a div, where it reads the contents of the div.  You have to tell VO to navigate inside the frame to read the contents.

I am unfortunately not familiar enough with WebCore's AX code to fix this.  I looked around to see if there was an easy way to change the role of the frame, but it seems to be handled by a separate system from the AX roles.  Someone with AX knowledge will have to comment. :)
Comment 2 chris fleizach 2012-05-14 22:53:02 PDT
When determining the AX role we'd check for this attribute and return a GroupRole instead. We'd then need to make sure that the code the connects frames to their parent documents in the AX hierarchy didn't break because the frame no longer identified as a Web Area. We'd also want to hide the Scroll Area parent that probably exists in the AX hierarchy (but is not visible)
Comment 3 chris fleizach 2012-05-14 22:54:37 PDT
Comment 4 Eric Seidel (no email) 2012-05-14 23:44:21 PDT
The only bit that you'll need to check is HTMLIFrameElement::shouldDisplaySeamlesslyWithParent() or Document::shouldDisplaySeamlesslyWithParent().  Both return the same thing but can be asked from either side (depending which pointer you happen to have).

That single bool-returning function controls the entire operation of seamless.   Assuming the AX tree is sensitive to rendering tree changes, it should notice any dynamic update of seamless, but I'm happy to help you write tests for that if needed.
Comment 5 chris fleizach 2012-12-16 23:27:30 PST
Created attachment 179692 [details]
Comment 6 WebKit Review Bot 2012-12-16 23:30:47 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 7 WebKit Review Bot 2012-12-16 23:44:21 PST
Comment on attachment 179692 [details]

Attachment 179692 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15378558
Comment 8 Eric Seidel (no email) 2012-12-16 23:47:53 PST
Comment on attachment 179692 [details]

OK.  I thought there was a FIXME in the code somewhere?  Or an existing bug... maybe we just discussed this over IRC.  <iframe seamless> is in a 95%-shippable state.  We are discussing disabling it for Chrome, as it's currently w/o an owner and needs a few bug fixes before it's robust enough for adoption.  This is still a good fix to have regardless, of course. :)

Looks like you got a cr-linux failure, btw.
Comment 9 chris fleizach 2012-12-16 23:49:00 PST
Created attachment 179693 [details]
Comment 10 Peter Beverloo (cr-android ews) 2012-12-16 23:55:59 PST
Comment on attachment 179692 [details]

Attachment 179692 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15361692
Comment 11 Eric Seidel (no email) 2012-12-17 00:11:40 PST
Nm, this was the bug I was thinking about.
Comment 12 Build Bot 2012-12-17 01:22:11 PST
Comment on attachment 179693 [details]

Attachment 179693 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15375631

New failing tests:
Comment 13 chris fleizach 2012-12-17 09:09:04 PST
Created attachment 179756 [details]
patch for landing
Comment 14 chris fleizach 2012-12-17 17:23:16 PST