Accessibility client cannot navigate to internal links targets on iOS.
Created attachment 375884 [details] Patch
Comment on attachment 375884 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375884&action=review Looks good. > Source/WebCore/accessibility/AccessibilityObject.cpp:545 > + return WebCore::firstAccessibleObjectFromNode(node, [] (const AccessibilityObject& acc) { > + return !acc.accessibilityIsIgnored(); > + }); WebKit coding style says we should use words, not abbreviations, for variable names. So the name here could be "object", "accessibilityObject", "candidate", or some other word, but shouldn’t be "acc". > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1816 > + /* AccessibilityObject::linkedUIElements may return an object that is > + exposed in other platforms but not on iOS, i.e., grouping or structure > + elements like <div> or <p>. Thus find the next accessible object that is > + exposed on iOS. > + */ WebKit coding style calls for "//" comments, not "/*" comments. > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1820 > + if (auto linked = firstAccessibleObjectFromNode(linkedElements[0]->node(), [] (const AccessibilityObject& acc) { > + return acc.wrapper().isAccessibilityElement; > + })) > + return linked->wrapper(); Same issue with variable named "acc". Formatting here makes this hard to read. Might want to rearrange to avoid that. One way to do it is: auto linkedElement = firstAccessibleObjectFromNode(linkedElements[0]->node(), [] (const AccessibilityObject& object) { return object.wrapper().isAccessibilityElement; }); return linkedElement ? linkedElement->wrapper() : nullptr; But I’m sure there are other ways to sidestep it too.
Created attachment 375944 [details] Patch
(In reply to Darin Adler from comment #2) > Comment on attachment 375884 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=375884&action=review > > Looks good. > Thanks very much Darin, for the prompt review and good points. > > Source/WebCore/accessibility/AccessibilityObject.cpp:545 > > + return WebCore::firstAccessibleObjectFromNode(node, [] (const AccessibilityObject& acc) { > > + return !acc.accessibilityIsIgnored(); > > + }); > > WebKit coding style says we should use words, not abbreviations, for > variable names. So the name here could be "object", "accessibilityObject", > "candidate", or some other word, but shouldn’t be "acc". > Changed it too accessible. > > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1816 > > + /* AccessibilityObject::linkedUIElements may return an object that is > > + exposed in other platforms but not on iOS, i.e., grouping or structure > > + elements like <div> or <p>. Thus find the next accessible object that is > > + exposed on iOS. > > + */ > > WebKit coding style calls for "//" comments, not "/*" comments. > Changed it to // comment. I personally prefer /* */ for multiline comments, so that I don't have to hear "slash slash" before every line of text. But I will of course adhere to the WebKit coding style conventions. > > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1820 > > + if (auto linked = firstAccessibleObjectFromNode(linkedElements[0]->node(), [] (const AccessibilityObject& acc) { > > + return acc.wrapper().isAccessibilityElement; > > + })) > > + return linked->wrapper(); > > Same issue with variable named "acc". > > Formatting here makes this hard to read. Might want to rearrange to avoid > that. One way to do it is: > > auto linkedElement = > firstAccessibleObjectFromNode(linkedElements[0]->node(), [] (const > AccessibilityObject& object) { > return object.wrapper().isAccessibilityElement; > }); > return linkedElement ? linkedElement->wrapper() : nullptr; > > But I’m sure there are other ways to sidestep it too. Thanks! agree this way is much more legible. Done.
Comment on attachment 375944 [details] Patch Clearing flags on attachment: 375944 Committed r248513: <https://trac.webkit.org/changeset/248513>
All reviewed patches have been landed. Closing bug.
<rdar://problem/54173330>