All the examples from http://www.html5accessibility.com/tests/hidden2013.html should work. Right now, cases 5 and 6 fail <p id="e" hidden aria-hidden="false">test html5 hidden</p> <p id="f" style="display:none" aria-hidden="false">test display:none</p> Because aria-hidden=false is only applying to the element itself, not the sub-tree below itself
<rdar://problem/15574925>
Created attachment 218984 [details] patch
*** Bug 125591 has been marked as a duplicate of this bug. ***
Note that making aria-hidden override CSS display:none is controversial over in Mozilla land: https://bugzilla.mozilla.org/show_bug.cgi?id=945194
Comment on attachment 218984 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=218984&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:980 > + for (Node* testNode = node; testNode; testNode = testNode->parentNode()) { Maybe a small comment here indicating that you are doing this to be able to work with subtrees would be helpful > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:417 > + if (m_node->parentNode() && m_node->parentNode()->hasTagName(iframeTag) && m_node->parentNode()->renderer()) > + return true; Perhaps it would be useful to add a check in the layout test for this situation with iframes, given that you are adding this explicit check here > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:420 > + // Whitespace only text elements should be ignored when they have no renderer. > + String string = stringValue().stripWhiteSpace().simplifyWhiteSpace(); Shouldn't you be checking that the object has no renderer, as you mention in the comment? > LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html:22 > + Extra line > LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html:23 > + description("This tests that setting a role on an img still allows the alt attribute to be used for the description.."); It looks like this description is not the right one > LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html:39 > + Extra line
Created attachment 219219 [details] patch
Comment on attachment 219219 [details] patch Attachment 219219 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/49108065 New failing tests: accessibility/aria-hidden-false-works-in-subtrees.html
Created attachment 219244 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 219430 [details] patch
Comment on attachment 219430 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=219430&action=review Lgtm. I've just have a suggestion to replace shouldBe() with debug(), see it below... > LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html:35 > + shouldBe("iframeChild.role", "'AXRole: AXGroup'"); Could you replace this shouldBe() with a debug() line before landing, to make it more cross-platform friendly?
(In reply to comment #10) > (From update of attachment 219430 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=219430&action=review > > Lgtm. I've just have a suggestion to replace shouldBe() with debug(), see it below... > > > LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html:35 > > + shouldBe("iframeChild.role", "'AXRole: AXGroup'"); > > Could you replace this shouldBe() with a debug() line before landing, to make it more cross-platform friendly? yep no problem
https://trac.webkit.org/changeset/160789