[Webkit-unassigned] [Bug 265952] AX: insertionPointLineNumber hits the main thread even for single-line text fields

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=265952

--- Comment #4 from Andres Gonzalez <andresg_22 at apple.com> ---
(In reply to Dominic Mazzoni from comment #2)
> Created attachment 468915 [details]
> Patch

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
index 9bcd149f3594..5a6835b38f32 100644
--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
+++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
@@ -168,7 +168,6 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb
     setProperty(AXPropertyName::IsKeyboardFocusable, object.isKeyboardFocusable());
     setProperty(AXPropertyName::BrailleRoleDescription, object.brailleRoleDescription().isolatedCopy());
     setProperty(AXPropertyName::BrailleLabel, object.brailleLabel().isolatedCopy());
-    setProperty(AXPropertyName::IsNonNativeTextControl, object.isNonNativeTextControl());

     RefPtr geometryManager = tree()->geometryManager();
     std::optional frame = geometryManager ? geometryManager->cachedRectForID(object.objectID()) : std::nullopt;
@@ -352,6 +351,10 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb
         auto range = object.textInputMarkedTextMarkerRange();
         if (auto characterRange = range.characterRange(); range && characterRange)
             setProperty(AXPropertyName::TextInputMarkedTextMarkerRange, std::pair<AXID, CharacterRange>(range.start().objectID(), *characterRange));
+
+        bool isNonNativeTextControl = object.isNonNativeTextControl();
+        setProperty(AXPropertyName::IsNonNativeTextControl, isNonNativeTextControl);
+        setProperty(AXPropertyName::IsSingleLineTextField, computeIsSingleLineTextField(object, isNonNativeTextControl));
     }

     // These properties are only needed on the AXCoreObject interface due to their use in ATSPI,
@@ -371,6 +374,18 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb
     initializePlatformProperties(axObject);
 }

+bool AXIsolatedObject::computeIsSingleLineTextField(AccessibilityObject& object, bool isNonNativeTextControl)
+{
+    if (isNonNativeTextControl)
+        return object.hasAttribute(aria_multilineAttr) && !object.ariaIsMultiline();
+
+    if (auto *renderer = object.renderer(); renderer->isRenderTextControl())

AG: style: auto *renderer should be auto* renderer
AG: need to check renderer for null. This is why I don't like this compound if statement syntax, if you had the expression to the left of the ;, that would be a check for null. But since you have a second expression, the first one is just initialization and the second is the actual check.

+        return !renderer->isRenderTextControlMultiLine();
+
+    // If we're not sure, return false, it means we can't use this as an optimization to avoid computing the line index.
+    return false;
+}

AG: it would be clearer to have this method return an enum, something like enum class : uint_8 { Unknown, SingleLine, MultiLine }.

+
 AccessibilityObject* AXIsolatedObject::associatedAXObject() const
 {
     ASSERT(isMainThread());
@@ -1284,6 +1299,9 @@ bool AXIsolatedObject::isNativeTextControl() const

 int AXIsolatedObject::insertionPointLineNumber() const
 {
+    if (boolAttributeValue(AXPropertyName::IsSingleLineTextField))
+        return 0;
+
     return Accessibility::retrieveValueFromMainThread<int>([this] () -> int {
         if (auto* axObject = associatedAXObject())
             return axObject->insertionPointLineNumber();

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20231207/0b9f1f18/attachment-0001.htm>