Bug 35986

Summary: VoiceOver can navigate to hidden content in widget (WAI-ARIA)
Product: WebKit Reporter: chris fleizach <cfleizach>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, cfleizach, eric, gustavo, jdiggs, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch bdakin: review+

chris fleizach
Reported 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
Attachments
Patch (20.63 KB, patch)
2010-03-10 14:37 PST, chris fleizach
bdakin: review+
chris fleizach
Comment 1 2010-03-10 14:37:22 PST
Beth Dakin
Comment 2 2010-03-10 14:51:23 PST
Comment on attachment 50439 [details] Patch 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?
chris fleizach
Comment 3 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 accessibilityIgnoredBase baselineAccessibilityIgnored accessibillityIgnoredBaseline
Beth Dakin
Comment 4 2010-03-10 16:15:18 PST
accessibilityIgnoredBase I think I like this one best.
chris fleizach
Comment 5 2010-03-10 16:36:37 PST
Beth Dakin
Comment 7 2010-03-10 23:22:23 PST
Looks like this caused a test failure on Leopard, Chris :-(
chris fleizach
Comment 8 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
Gustavo Noronha (kov)
Comment 9 2010-03-11 11:19:24 PST
This change makes two regressions tests fail on GTK+: accessibility/aria-hidden-with-elements.html platform/gtk/accessibility/table-hierarchy.html 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.
chris fleizach
Comment 10 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
chris fleizach
Comment 11 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
chris fleizach
Comment 12 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
chris fleizach
Comment 13 2010-03-11 11:59:38 PST
Gustavo Noronha (kov)
Comment 14 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.
chris fleizach
Comment 15 2010-03-11 12:37:44 PST
i'm uploading a patch now
Note You need to log in before you can comment on or make changes to this bug.