Add a simple way to retrieve the focusable or editable element containing an element (possibly that element itself) in order to better make decisions about notifying AT users about focus changes.
<rdar://problem/25198311>
Created attachment 274211 [details] Patch
Comment on attachment 274211 [details] Patch Attachment 274211 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/990314 New failing tests: accessibility/plugin.html accessibility/mac/bounds-for-range.html accessibility/image-link.html accessibility/table-attributes.html accessibility/mac/internal-link-anchors.html accessibility/mac/document-links.html accessibility/table-sections.html accessibility/math-multiscript-attributes.html accessibility/table-one-cell.html accessibility/table-detection.html accessibility/table-cells.html accessibility/image-map2.html accessibility/table-cell-spans.html accessibility/table-with-rules.html accessibility/transformed-element.html accessibility/mac/aria-columnrowheaders.html accessibility/internal-link-anchors2.html accessibility/lists.html
Created attachment 274213 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 274211 [details] Patch Attachment 274211 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/990326 New failing tests: accessibility/plugin.html accessibility/mac/bounds-for-range.html accessibility/image-link.html accessibility/table-attributes.html accessibility/mac/internal-link-anchors.html accessibility/mac/document-links.html accessibility/table-sections.html accessibility/math-multiscript-attributes.html accessibility/table-one-cell.html accessibility/table-detection.html accessibility/table-cells.html accessibility/image-map2.html accessibility/table-cell-spans.html accessibility/table-with-rules.html accessibility/transformed-element.html accessibility/mac/aria-columnrowheaders.html accessibility/internal-link-anchors2.html accessibility/lists.html
Created attachment 274215 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 274211 [details] Patch Attachment 274211 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/990334 New failing tests: accessibility/plugin.html accessibility/mac/bounds-for-range.html accessibility/image-link.html accessibility/table-attributes.html accessibility/mac/internal-link-anchors.html accessibility/mac/document-links.html accessibility/table-sections.html accessibility/math-multiscript-attributes.html accessibility/table-one-cell.html accessibility/table-detection.html accessibility/table-cells.html accessibility/image-map2.html accessibility/table-cell-spans.html accessibility/table-with-rules.html accessibility/transformed-element.html accessibility/mac/aria-columnrowheaders.html accessibility/internal-link-anchors2.html accessibility/lists.html
Created attachment 274218 [details] Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 274244 [details] Patch
(In reply to comment #9) > Created attachment 274244 [details] > Patch what is the purpose of this patch? what will it be used for? this looks like it also add a lot of extra cycles to web retrieval, if these are now three new attributes on every element, that will run up the parent chain independently for every call
(In reply to comment #10) > (In reply to comment #9) > > Created attachment 274244 [details] > > Patch > > what is the purpose of this patch? what will it be used for? > > this looks like it also add a lot of extra cycles to web retrieval, if these > are now three new attributes on every element, that will run up the parent > chain independently for every call The purpose is to simplify the logic that decides if a focus change should be alerted to a user or if we've simply moved between focusable subelements.
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > Created attachment 274244 [details] > > > Patch > > > > what is the purpose of this patch? what will it be used for? > > > > this looks like it also add a lot of extra cycles to web retrieval, if these > > are now three new attributes on every element, that will run up the parent > > chain independently for every call > > The purpose is to simplify the logic that decides if a focus change should > be alerted to a user or if we've simply moved between focusable subelements. and that is happening inside VoiceOver now? Will VO fetch these on demand or automatically for all elements?
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > (In reply to comment #9) > > > > Created attachment 274244 [details] > > > > Patch > > > > > > what is the purpose of this patch? what will it be used for? > > > > > > this looks like it also add a lot of extra cycles to web retrieval, if these > > > are now three new attributes on every element, that will run up the parent > > > chain independently for every call > > > > The purpose is to simplify the logic that decides if a focus change should > > be alerted to a user or if we've simply moved between focusable subelements. > > and that is happening inside VoiceOver now? VoiceOver currently uses some unreliable heuristics based on roles and frames that this is meant to replace. > > Will VO fetch these on demand or automatically for all elements? Only on demand. I think the only place where this is adding overhead is in the list of supported attributes growing by three entries and a couple more if statements in accessibilityAttributeValue
Comment on attachment 274244 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=274244&action=review some review comments inside > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:253 > + if (canSetFocusAttribute()) if you start your loop with "this" you can avoid this extra bit of code with no extra cost > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:268 > + if (isTextControl()) ditto > Source/WebCore/accessibility/AccessibilityObject.h:1043 > these seem like they can all go in AccessibilityObject.cpp and then they don't have to be virtual > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3335 > + if (!m_object->isWebArea()) { if its called on WebArea, it will just end up returning nil right? that seems ok doesn't it
(In reply to comment #14) > Comment on attachment 274244 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=274244&action=review > > some review comments inside > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:253 > > + if (canSetFocusAttribute()) > > if you start your loop with "this" you can avoid this extra bit of code with > no extra cost Will do > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:268 > > + if (isTextControl()) > > ditto Will do > > > Source/WebCore/accessibility/AccessibilityObject.h:1043 > > > > these seem like they can all go in AccessibilityObject.cpp and then they > don't have to be virtual I'll see if I can do that. > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3335 > > + if (!m_object->isWebArea()) { > > if its called on WebArea, it will just end up returning nil right? that > seems ok doesn't it Correct. I can remove that check.
Created attachment 274323 [details] Patch
Comment on attachment 274323 [details] Patch Clearing flags on attachment: 274323 Committed r198356: <http://trac.webkit.org/changeset/198356>
All reviewed patches have been landed. Closing bug.