Bug 35986 - VoiceOver can navigate to hidden content in widget (WAI-ARIA)
Summary: VoiceOver can navigate to hidden content in widget (WAI-ARIA)
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: chris fleizach
Depends on:
Reported: 2010-03-10 14:32 PST by chris fleizach
Modified: 2010-03-11 12:37 PST (History)
6 users (show)

See Also:

Patch (20.63 KB, patch)
2010-03-10 14:37 PST, chris fleizach
bdakin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2010-03-10 14:32:06 PST
VoiceOver can navigate to hidden content because aria-hidden is not respected on elements don't use AccessibilityRenderObject::accessibilityIsIgnored
Comment 1 chris fleizach 2010-03-10 14:37:22 PST
Created attachment 50439 [details]
Comment 2 Beth Dakin 2010-03-10 14:51:23 PST
Comment on attachment 50439 [details]

Generally, I think this patch is good, so r=me. However, I don't like having two functions called accessibilityIsIgnored() and accessibilityIsIgnoredCommon(). I wish the names of these functions made the differences between the two more obvious. Maybe we could think of better names. Chris, could you explain what you think the difference is between the two, and perhaps that will inspire better names?
Comment 3 chris fleizach 2010-03-10 15:17:52 PST
so AccessibilityRenderObject ::accessibilityIsIgnored() has all kinds of logic in it to deal with the various roles that AccessibilityRenderObject comprises (web areas, text fields, links, images, etc). that logic would not work and be wasted if it was called by AccessibilityTable, which has its own logic to determine ignoring.

however, both those cases need to know some common things that would apply to any elements. namely, aria-hidden, presentational status and whether the platform says an element should be ignored.

so there's "common" ignored logic and then "AccessibilityRenderObject" specific ignored logic.

i had the hardest time with the naming too... so i don't have any great ideas. here are some that came to mind

Comment 4 Beth Dakin 2010-03-10 16:15:18 PST

I think I like this one best.
Comment 5 chris fleizach 2010-03-10 16:36:37 PST
Comment 7 Beth Dakin 2010-03-10 23:22:23 PST
Looks like this caused a test failure on Leopard, Chris :-(
Comment 8 chris fleizach 2010-03-11 06:41:41 PST
i had fixed those test failures by the time of your comment i'm pretty sure. things should be ok by now
Comment 9 Gustavo Noronha (kov) 2010-03-11 11:19:24 PST
This change makes two regressions tests fail on GTK+:


The first one looks like a new test, so failing it may not be bad, but the second one seems to be a real regression.
Comment 10 chris fleizach 2010-03-11 11:21:12 PST
it does look like a real regression. i'll take a close look at the diffs and see if i mismanaged a merge
Comment 11 chris fleizach 2010-03-11 11:27:17 PST
yea it looks like i made AccessibilityTableRow return the wrong thing on GTK. i'll fix this post-haste
Comment 12 chris fleizach 2010-03-11 11:58:44 PST
problem is that accessibilityIsIgnoredBase() no longer accounts for when the platform says to include the object. will make a new bug to change this behavior
Comment 13 chris fleizach 2010-03-11 11:59:38 PST
Comment 14 Gustavo Noronha (kov) 2010-03-11 12:35:16 PST
(In reply to comment #13)
> https://bugs.webkit.org/show_bug.cgi?id=36025

Thanks! Is this something you will be fixing soon, you think? If so, I think skipping the test for now, and unskipping it then is OK, but we should not leave this regression as is if it takes a while to correct, since we are planning a release in a few days, and should consider applying the usual policy of reverting on regressions.
Comment 15 chris fleizach 2010-03-11 12:37:44 PST
i'm uploading a patch now