This causes string comparisons to fail unexpectedly not match here: if (backingObject->isStyleFormatGroup()) { using namespace HTMLNames; auto tagName = backingObject->tagName(); if (tagName == kbdTag) return @"AXKeyboardInputStyleGroup"; if (tagName == codeTag) return @"AXCodeStyleGroup"; if (tagName == preTag) return @"AXPreformattedStyleGroup"; if (tagName == sampTag) return @"AXSampleStyleGroup"; if (tagName == varTag) return @"AXVariableStyleGroup"; if (tagName == citeTag) return @"AXCiteStyleGroup"; ASSERT_NOT_REACHED(); }
<rdar://problem/87842864>
Created attachment 449601 [details] Patch
Created attachment 449605 [details] Patch
Comment on attachment 449605 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449605&action=review > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2020 > + return Accessibility::retrieveValueFromMainThread<String>([protectedSelf = retainPtr(self)] () -> String { should we cache this rather than hitting main thread?
Created attachment 449638 [details] Patch
Created attachment 449639 [details] Patch
Comment on attachment 449639 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449639&action=review > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1807 > + && (!backingObject->isModel() || backingObject->modelElementChildren().isEmpty()) I think even if the model is empty we want to return the special subrole so that VO can land on it directly and know that it's a model
Created attachment 449658 [details] Patch
(In reply to chris fleizach from comment #7) > Comment on attachment 449639 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449639&action=review > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1807 > > + && (!backingObject->isModel() || backingObject->modelElementChildren().isEmpty()) > > I think even if the model is empty we want to return the special subrole so > that VO can land on it directly and know that it's a model Fixed.
(In reply to Tyler Wilcock from comment #8) > Created attachment 449658 [details] > Patch --- a/Source/WebCore/accessibility/AccessibilityObject.cpp +++ a/Source/WebCore/accessibility/AccessibilityObject.cpp +String AccessibilityObject::subrolePlatformString() const +{ + return String(); +} + I think we do these hallowed implementations as inlines in the header. --- a/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm +++ a/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm + setProperty(AXPropertyName::SubrolePlatformString, object.subrolePlatformString().isolatedCopy()); We can do this in the cross-platform initializeData since it is the same for all platforms. --- a/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm +++ a/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm +String AccessibilityObject::subrolePlatformString() const +{ This can not be implemented twice in AccessibilityObject.cpp and Mac.mm. --- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm +++ a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm + if (subrole != String()) + return subrole; Change to if (!subrole.isEmpty())
Created attachment 449684 [details] Patch
> +String AccessibilityObject::subrolePlatformString() const > +{ > + return String(); > +} > + > I think we do these hallowed implementations as inlines in the header. > --- a/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm > +++ a/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm > > +String AccessibilityObject::subrolePlatformString() const > +{ > > This can not be implemented twice in AccessibilityObject.cpp and Mac.mm. The patch diff doesn't show enough context for you to have seen this, but it's not implemented twice. I added the hollow implementation of subrolePlatformString to the .cpp source file to match how we do it for roles, as that is already wrapped in an #if !PLATFORM(MAC) guard: #if !PLATFORM(MAC) String AccessibilityObject::rolePlatformString() const { return Accessibility::roleToPlatformString(roleValue()); } String AccessibilityObject::rolePlatformDescription() const { // FIXME: implement in other platforms. return String(); } String AccessibilityObject::subrolePlatformString() const { return String(); } #endif Then we provide the real implementation in AccessibilityObjectMac.mm. I can still change it if you'd like, though. Fixed your other two comments, thanks!
Created attachment 449686 [details] Patch
Committed r288390 (246287@main): <https://commits.webkit.org/246287@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449686 [details].