Bug 179495 – AX: AOM: Implement string type properties

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Nan Wang]

AOM Phase 1:
Implement all of the string type properties.

Spec: https://wicg.github.io/aom/spec/

<rdar://problems/35077884>

[Nan Wang]

Created attachment 327101 [details]
patch

[chris fleizach]

Comment on attachment 327101 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327101&action=review

> Source/WebCore/accessibility/AccessibilityObject.cpp:2500
> +    const AtomicString& ariaPlaceholder = stringValueForProperty(AXPropertyName::Placeholder);

if this is still atomic why did the return type change?

> Source/WebCore/accessibility/AccessibilityObject.h:704
> +    const String ariaKeyShortcutsValue() const;

should we remove aria as a prefix since this does AOM and aria

> Source/WebCore/accessibility/AccessibilityObject.h:985
> +    virtual const String ariaLiveRegionRelevant() const { return nullAtom(); }

ditto about these method names with aria

[Nan Wang]

Comment on attachment 327101 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327101&action=review

>> Source/WebCore/accessibility/AccessibilityObject.cpp:2500
>> +    const AtomicString& ariaPlaceholder = stringValueForProperty(AXPropertyName::Placeholder);
> 
> if this is still atomic why did the return type change?

Because stringValueForProperty() is not returning the reference, so here if we don't change it would be error that returning reference to local temporary object.
const AtomicString& ariaPlaceholder = stringValueForProperty(AXPropertyName::Placeholder); is to prolong the lifetime of the String object within this function.
I tried to change stringValueForProperty() by returning a reference, but getting the string value that's being set from JS would hit the assertion where "The atomic string comes from an other thread!"

>> Source/WebCore/accessibility/AccessibilityObject.h:704
>> +    const String ariaKeyShortcutsValue() const;
> 
> should we remove aria as a prefix since this does AOM and aria

Ok

>> Source/WebCore/accessibility/AccessibilityObject.h:985
>> +    virtual const String ariaLiveRegionRelevant() const { return nullAtom(); }
> 
> ditto about these method names with aria

Ok

[Nan Wang]

Created attachment 327129 [details]
patch

Removed aria prefix

[WebKit Commit Bot]

Comment on attachment 327129 [details]
patch

Clearing flags on attachment: 327129

Committed r224953: <https://trac.webkit.org/changeset/224953>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.