Summary: | AX: AOM: Implement string type properties | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nan Wang <n_wang> | ||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, buildbot, cdumez, cfleizach, commit-queue, dmazzoni, esprehn+autocc, ews-watchlist, jcraig, jdiggs, joepeck, kondapallykalyan, n_wang, rniwa, samuel_white, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=179255 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 179255 | ||||||||
Attachments: |
|
Description
Nan Wang
2017-11-09 11:27:52 PST
Created attachment 327101 [details]
patch
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 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 Created attachment 327129 [details]
patch
Removed aria prefix
Comment on attachment 327129 [details] patch Clearing flags on attachment: 327129 Committed r224953: <https://trac.webkit.org/changeset/224953> All reviewed patches have been landed. Closing bug. |