Summary: | AX: [Mac] better accessibility support for Summary elements | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nan Wang <n_wang> | ||||||||||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, mario, n_wang, samuel_white, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Attachments: |
|
Description
Nan Wang
2016-10-12 16:21:06 PDT
Created attachment 291415 [details]
patch
Comment on attachment 291415 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291415&action=review > Source/WebCore/accessibility/AccessibilityObject.cpp:2483 > + for (AccessibilityObject* parentObject = this->parentObject(); parentObject; parentObject = parentObject->parentObject()) { do we have an enclosure for this? > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1413 > + if (m_object->supportsExpanded() || m_object->isSummary()) should supportsExpanded() also include isSummary()? Comment on attachment 291415 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291415&action=review >> Source/WebCore/accessibility/AccessibilityObject.cpp:2483 >> + for (AccessibilityObject* parentObject = this->parentObject(); parentObject; parentObject = parentObject->parentObject()) { > > do we have an enclosure for this? we don't in this class. >> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1413 >> + if (m_object->supportsExpanded() || m_object->isSummary()) > > should supportsExpanded() also include isSummary()? I think it's the detail element that actually has the expanded status. I'm not sure how other platform is doing this, so putting here to only affect mac. Created attachment 291460 [details]
patch
Added the enclosure for finding the parent object
Comment on attachment 291460 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291460&action=review > Source/WebCore/accessibility/AccessibilityObject.cpp:463 > + return AccessibilityObject::matchedParent(const_cast<AccessibilityObject*>(this), false, matchFunc); can we write these enclosures inline to save space? return AccessibilityObject::matchedParent(const_cast<AccessibilityObject*>(this), false, [](AccessibilityObject* object) { 461 return !object->accessibilityIsIgnored(); 462 }); Created attachment 291461 [details]
patch
updated style
Attachment 291461 [details] did not pass style-queue:
ERROR: Source/WebCore/accessibility/AccessibilityObject.cpp:460: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/accessibility/AccessibilityObject.cpp:1696: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/accessibility/AccessibilityObject.cpp:1790: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/accessibility/AccessibilityObject.cpp:2315: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/accessibility/AccessibilityObject.cpp:2484: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/accessibility/AccessibilityObject.cpp:2945: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/accessibility/AccessibilityObject.cpp:3043: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/WebCore/accessibility/AccessibilityObject.cpp:3049: More than one command on the same line [whitespace/newline] [4]
Total errors found: 8 in 10 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 291462 [details]
patch
updated style again.
Created attachment 291464 [details]
patch
Oops.. missed something
Comment on attachment 291464 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291464&action=review > Source/WebCore/accessibility/AccessibilityObject.h:1066 > + static AccessibilityObject* matchedParent(AccessibilityObject*, bool includeSelf, const std::function<bool(AccessibilityObject*)>&); I don’t think the types are quite right here. I think we should probably use const versions and take references. The passed in function should take an AccesibilityObject&, not an AccessibilityObject*. static const AccessibilityObject* matchedParent(const AccessibilityObject&, bool includeSelf, const std::function<bool(const AccessibilityObject&)>&); And if we really need a non-const version (I am not sure we do): static AccessibilityObject* matchedParent(AccessibilityObject&, bool includeSelf, const std::function<bool(const AccessibilityObject&)>&); Created attachment 291515 [details]
patch for landing
Updated the types based on the review.
Changed lots of code so uploaded a patch to see if everything still works.
Comment on attachment 291515 [details] patch for landing Clearing flags on attachment: 291515 Committed r207314: <http://trac.webkit.org/changeset/207314> All reviewed patches have been landed. Closing bug. |