Bug 264642

<rdar://problem/118255218>

Created attachment 468566 [details] Patch

Created attachment 468567 [details] Patch

Comment on attachment 468567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468567&action=review > Source/WebCore/accessibility/AXCoreObject.h:678 > + AccessibilityText(const String& t, const AccessibilityTextSource& s) > + : text(t) > + , textSource(s) I know you're only copy-pasting this, but maybe we can change it to follow the style guidelines and give these proper names, e.g. `text` and `source` > Source/WebCore/accessibility/AccessibilityObject.h:121 > + bool isFileUploadButton() const override; Can this be final? > Source/WebCore/accessibility/AccessibilityObject.h:190 > + bool isMeter() const override; Can this be final? > Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:72 > + Vector<AccessibilityText> texts; > + object->accessibilityText(texts); This may be a good opportunity to make AXCoreObject::accessibilityText return a Vector<AccessibilityText>, rather than returning void and taking in a Vector<AccessibilityText>&. Although that's not related to the intention of your patch, so if you wanted to skip it that's OK too. > Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:75 > + auto axTextValue = texts.map([] (const auto& text) -> AccessibilityText { > + return { text.text.isolatedCopy(), text.textSource }; > + }); This is an unresearched idea...but is there some text sources we can filter out before caching AccessibilityText to save memory? We should be caching the minimum to correctly serve title, description, and help text. e.g., if an object computes text with AccessibilityTextSource::Subtitle, is that needed in any way to compute any of the three properties in question?

Comment on attachment 468567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468567&action=review >> Source/WebCore/accessibility/AXCoreObject.h:678 >> + , textSource(s) > > I know you're only copy-pasting this, but maybe we can change it to follow the style guidelines and give these proper names, e.g. `text` and `source` Yeah, I'll go ahead and update those. >> Source/WebCore/accessibility/AccessibilityObject.h:190 >> + bool isMeter() const override; > > Can this be final? yes to both of these. >> Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:75 >> + }); > > This is an unresearched idea...but is there some text sources we can filter out before caching AccessibilityText to save memory? We should be caching the minimum to correctly serve title, description, and help text. e.g., if an object computes text with AccessibilityTextSource::Subtitle, is that needed in any way to compute any of the three properties in question? I think that's a great call out! I will audit what text sources we actually need for those.

Comment on attachment 468567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468567&action=review >> Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:72 >> + object->accessibilityText(texts); > > This may be a good opportunity to make AXCoreObject::accessibilityText return a Vector<AccessibilityText>, rather than returning void and taking in a Vector<AccessibilityText>&. Although that's not related to the intention of your patch, so if you wanted to skip it that's OK too. Yeah we should definitely do this. I'll break this into a follow-up patch since it will be a significant change (we'll also want to update titleElementText, alternativeText, visibleText, and helpText to return a vector too). >>> Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm:75 >>> + }); >> >> This is an unresearched idea...but is there some text sources we can filter out before caching AccessibilityText to save memory? We should be caching the minimum to correctly serve title, description, and help text. e.g., if an object computes text with AccessibilityTextSource::Subtitle, is that needed in any way to compute any of the three properties in question? > > I think that's a great call out! I will audit what text sources we actually need for those. Unfortunately we use all of the different sources for calculating Title/Description/Help, so we can't do any filtering here.

Created attachment 468604 [details] Patch

Created attachment 468611 [details] Patch

Created attachment 468616 [details] Patch

Committed 270860@main (ef43e42c5970): <https://commits.webkit.org/270860@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 468616 [details].