Bug 86317

Summary: Seamless iframe should not announce a new browsing context
Product: WebKit Reporter: Lars Gunther <webmaster>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
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    
Attachments:
Description Flags
patch
eric: review+, webkit.review.bot: commit-queue-
patch
buildbot: commit-queue-
patch for landing none

Lars Gunther
Reported 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
Attachments
patch (11.80 KB, patch)
2012-12-16 23:27 PST, chris fleizach 		eric: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
patch (11.81 KB, patch)
2012-12-16 23:49 PST, chris fleizach 		buildbot: commit-queue-
DetailsFormatted DiffDiff
patch for landing (11.69 KB, patch)
2012-12-17 09:09 PST, chris fleizach 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 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. :)
chris fleizach
Comment 2 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)
chris fleizach
Comment 3 2012-05-14 22:54:37 PDT
rdar://11452561
Eric Seidel (no email)
Comment 4 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.
chris fleizach
Comment 5 2012-12-16 23:27:30 PST
Created attachment 179692 [details] patch
WebKit Review Bot
Comment 6 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.
WebKit Review Bot
Comment 7 2012-12-16 23:44:21 PST
Comment on attachment 179692 [details] patch Attachment 179692 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15378558
Eric Seidel (no email)
Comment 8 2012-12-16 23:47:53 PST
Comment on attachment 179692 [details] patch 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.
chris fleizach
Comment 9 2012-12-16 23:49:00 PST
Created attachment 179693 [details] patch
Peter Beverloo (cr-android ews)
Comment 10 2012-12-16 23:55:59 PST
Comment on attachment 179692 [details] patch Attachment 179692 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/15361692
Eric Seidel (no email)
Comment 11 2012-12-17 00:11:40 PST
Nm, this was the bug I was thinking about.
Build Bot
Comment 12 2012-12-17 01:22:11 PST
Comment on attachment 179693 [details] patch Attachment 179693 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15375631 New failing tests: platform/mac/accessibility/seamless-iframe.html
chris fleizach
Comment 13 2012-12-17 09:09:04 PST
Created attachment 179756 [details] patch for landing
chris fleizach
Comment 14 2012-12-17 17:23:16 PST
http://trac.webkit.org/changeset/137962
Note You need to log in before you can comment on or make changes to this bug.