Bug 86317 - Seamless iframe should not announce a new browsing context
Summary: Seamless iframe should not announce a new browsing context
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks: 45950
  Show dependency treegraph
 
Reported: 2012-05-13 06:59 PDT by Lars Gunther
Modified: 2012-12-17 17:23 PST (History)
12 users (show)

See Also:


Attachments
patch (11.80 KB, patch)
2012-12-16 23:27 PST, chris fleizach
eric: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
patch (11.81 KB, patch)
2012-12-16 23:49 PST, chris fleizach
buildbot: commit-queue-
Details | Formatted Diff | Diff
patch for landing (11.69 KB, patch)
2012-12-17 09:09 PST, chris fleizach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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
rdar://11452561
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]
patch
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]
patch

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]
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.
Comment 9 chris fleizach 2012-12-16 23:49:00 PST
Created attachment 179693 [details]
patch
Comment 10 Peter Beverloo (cr-android ews) 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
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]
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
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
http://trac.webkit.org/changeset/137962