AX: Extremely long textareas/contenteditable fields are slow with Voi…

…ceover https://bugs.webkit.org/show_bug.cgi?id=263303 rdar://problem/117114220 Reviewed by Tyler Wilcock. Very long textareas/contenteditable fields can be slow with AT due to two expensive operations in WebKit: (1) Spellchecking long text & drawing misspellings. (2) Caching text using rangeForCharacterRange on the isolated tree. This patch addresses those two causes by: (1) Not spellchecking on the WebKit side when Voiceover is the AX client. (2) Only caching textarea/contenteditable values with fewer than 12,500 characters. There are also two tests included in this patch. The first verifies that not caching input values greater than 12,500 does not change behavior and the second confirms that we do not spellcheck within WebKit when VoiceOver is the AX client. * LayoutTests/accessibility/mac/large-text-area-expected.txt: Added. * LayoutTests/accessibility/mac/large-text-area.html: Added. * LayoutTests/accessibility/mac/spellcheck-with-voiceover-expected.txt: Added. * LayoutTests/accessibility/mac/spellcheck-with-voiceover.html: Added. * LayoutTests/platform/mac-wk1/TestExpectations: * Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm: (WebCore::AXIsolatedObject::initializePlatformProperties): * Source/WebCore/accessibility/mac/AXObjectCacheMac.mm: (WebCore::AXObjectCache::shouldSpellCheck): * Source/WebCore/editing/Editor.cpp: (WebCore::Editor::markMisspellingsAfterTypingToWord): (WebCore::Editor::markAllMisspellingsAndBadGrammarInRanges): (WebCore::Editor::markMisspellingsAndBadGrammar): * Source/WebCore/editing/TextCheckingHelper.cpp: (WebCore::TextCheckingHelper::findFirstMisspelledWordOrUngrammaticalPhrase const): (WebCore::TextCheckingHelper::guessesForMisspelledWordOrUngrammaticalPhrase const): (WebCore::platformOrClientDrivenTextCheckerEnabled): * Source/WebCore/editing/TextCheckingHelper.h: * Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h: * Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityController.idl: * Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityControllerAtspi.cpp: (WTR::AccessibilityController::overrideClient): * Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityControllerIOS.mm: (WTR::AccessibilityController::overrideClient): * Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityControllerMac.mm: (WTR::AccessibilityController::overrideClient): * Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp: (WTR::AccessibilityController::overrideClient): Canonical link: https://commits.webkit.org/270066@main
WebKit · Nov 1, 2023 · aa6898f · aa6898f
1 parent a3a7fc6
commit aa6898f
Show file tree

Hide file tree

Showing 16 changed files with 394 additions and 20 deletions.
diff --git a/LayoutTests/accessibility/mac/large-text-area-expected.txt b/LayoutTests/accessibility/mac/large-text-area-expected.txt
diff --git a/LayoutTests/accessibility/mac/large-text-area.html b/LayoutTests/accessibility/mac/large-text-area.html
diff --git a/LayoutTests/accessibility/mac/spellcheck-with-voiceover-expected.txt b/LayoutTests/accessibility/mac/spellcheck-with-voiceover-expected.txt
@@ -0,0 +1,46 @@
+This tests that when the AX client is VoiceOver, we do not spellcheck.
+
+Attributed String (without client set):
+Hello {
+    AXBackgroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 0 )";
+    AXFont =     {
+        AXFontFamily = ".AppleSystemUIFont";
+        AXFontName = ".SFNS-Regular";
+        AXFontSize = 11;
+    };
+    AXForegroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 1 )";
+}wolrd{
+    AXBackgroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 0 )";
+    AXFont =     {
+        AXFontFamily = ".AppleSystemUIFont";
+        AXFontName = ".SFNS-Regular";
+        AXFontSize = 11;
+    };
+    AXForegroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 1 )";
+    AXMarkedMisspelled = 1;
+    AXMisspelled = 1;
+}.{
+    AXBackgroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 0 )";
+    AXFont =     {
+        AXFontFamily = ".AppleSystemUIFont";
+        AXFontName = ".SFNS-Regular";
+        AXFontSize = 11;
+    };
+    AXForegroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 1 )";
+}
+Attributed String (with VoiceOver specified as the client):
+Hello wolrd!{
+    AXBackgroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 0 )";
+    AXDidSpellCheck = 0;
+    AXFont =     {
+        AXFontFamily = ".AppleSystemUIFont";
+        AXFontName = ".SFNS-Regular";
+        AXFontSize = 11;
+    };
+    AXForegroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 1 )";
+}
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/accessibility/mac/spellcheck-with-voiceover.html b/LayoutTests/accessibility/mac/spellcheck-with-voiceover.html
@@ -0,0 +1,40 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="../../resources/accessibility-helper.js"></script>
+</head>
+<body>
+
+<textarea id="textarea">Hello wolrd.</textarea>
+
+<script>
+let output = "This tests that when the AX client is VoiceOver, we do not spellcheck.\n\n";
+
+if (window.accessibilityController) {
+    window.jsTestIsAsync = true;
+    output += "Attributed String (without client set):\n"	
+    output += `${accessibilityController.accessibleElementById("textarea").attributedStringForRange(0, 12)}\n`;
+
+    output += "Attributed String (with VoiceOver specified as the client):\n"	
+    accessibilityController.overrideClient("voiceover");
+
+    // Change the text to clear the cached attributed string
+    document.getElementById("textarea").innerText = "Hello wolrd!";
+    setTimeout(async () => {
+        await waitFor(() => {
+            attributedString = accessibilityController.accessibleElementById("textarea").attributedStringForRange(0, 12);
+            return attributedString.indexOf("AXMisspelled = 1") == -1;
+        });
+        output += `${accessibilityController.accessibleElementById("textarea").attributedStringForRange(0, 12)}\n`;
+
+        // Reset client (important for stress test runs)
+        accessibilityController.overrideClient("");
+        debug(output);
+        finishJSTest();
+    })
+}
+</script>
+</body>
+</html>
+
diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -838,6 +838,7 @@ accessibility/display-contents/aria-owns.html [ Skip ]
 accessibility/mac/text-field-number-of-characters.html [ Skip ]
 accessibility/mac/content-editable-attributed-string.html [ Skip ]
 accessibility/mac/lazy-spellchecking.html [ Skip ]
+accessibility/mac/spellcheck-with-voiceover.html [ Skip ]
 accessibility/text-marker/text-marker-range-with-unordered-markers.html
 
 # DOM paste access requests are not implemented in WebKit1.

diff --git a/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm b/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm
@@ -40,22 +40,27 @@
     setProperty(AXPropertyName::SpeechHint, object->speechHintAttributeValue().isolatedCopy());
 
     RetainPtr<NSAttributedString> attributedText;
-    if (object->hasAttributedText()) {
-        std::optional<SimpleRange> range;
-        if (isTextControl())
-            range = rangeForCharacterRange({ 0, object->text().length() });
-        else
-            range = object->simpleRange();
-        if (range) {
-            if ((attributedText = object->attributedStringForRange(*range, SpellCheck::Yes)))
-                setProperty(AXPropertyName::AttributedText, attributedText);
+    // FIXME: Don't eagerly cache textarea/contenteditable values longer than 12500 characters as rangeForCharacterRange is very expensive.
+    // This should be cached once rangeForCharacterRange is more performant for large text control values.
+    unsigned textControlLength = isTextControl() ? object->text().length() : 0;
+    if (textControlLength < 12500) {
+        if (object->hasAttributedText()) {
+            std::optional<SimpleRange> range;
+            if (isTextControl())
+                range = rangeForCharacterRange({ 0, textControlLength });
+            else
+                range = object->simpleRange();
+            if (range) {
+                if ((attributedText = object->attributedStringForRange(*range, SpellCheck::Yes)))
+                    setProperty(AXPropertyName::AttributedText, attributedText);
+            }
         }
-    }
 
-    // Cache the TextContent only if it is not empty and differs from the AttributedText.
-    if (auto text = object->textContent()) {
-        if (!attributedText || (text->length() && *text != String([attributedText string])))
-            setProperty(AXPropertyName::TextContent, text->isolatedCopy());
+        // Cache the TextContent only if it is not empty and differs from the AttributedText.
+        if (auto text = object->textContent()) {
+            if (!attributedText || (text->length() && *text != String([attributedText string])))
+                setProperty(AXPropertyName::TextContent, text->isolatedCopy());
+        }
     }
 
     // Cache the StringValue only if it differs from the AttributedText.

diff --git a/Source/WebCore/accessibility/mac/AXObjectCacheMac.mm b/Source/WebCore/accessibility/mac/AXObjectCacheMac.mm
@@ -725,6 +725,10 @@ static bool isTestAXClientType(AXClientType client)
 
 bool AXObjectCache::shouldSpellCheck()
 {
+    // This method can be called from non-accessibility contexts, so we need to allow spellchecking if accessibility is disabled.
+    if (!accessibilityEnabled())
+        return true;
+
     if (UNLIKELY(forceDeferredSpellChecking()))
         return false;
 

diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
@@ -2679,7 +2679,7 @@ void Editor::markMisspellingsAfterTypingToWord(const VisiblePosition& wordStart,
 {
     Ref document = protectedDocument();
 
-    if (platformDrivenTextCheckerEnabled())
+    if (platformOrClientDrivenTextCheckerEnabled())
         return;
 
 #if PLATFORM(IOS_FAMILY)
@@ -2887,7 +2887,7 @@ void Editor::markBadGrammar(const VisibleSelection& selection)
 
 void Editor::markAllMisspellingsAndBadGrammarInRanges(OptionSet<TextCheckingType> textCheckingOptions, const std::optional<SimpleRange>& spellingRange, const std::optional<SimpleRange>& automaticReplacementRange, const std::optional<SimpleRange>& grammarRange)
 {
-    if (platformDrivenTextCheckerEnabled())
+    if (platformOrClientDrivenTextCheckerEnabled())
         return;
 
     ASSERT(unifiedTextCheckerEnabled());
@@ -3189,7 +3189,7 @@ void Editor::changeBackToReplacedString(const String& replacedString)
 
 void Editor::markMisspellingsAndBadGrammar(const VisibleSelection& spellingSelection, bool markGrammar, const VisibleSelection& grammarSelection)
 {
-    if (platformDrivenTextCheckerEnabled())
+    if (platformOrClientDrivenTextCheckerEnabled())
         return;
 
     if (unifiedTextCheckerEnabled()) {

diff --git a/Source/WebCore/editing/TextCheckingHelper.cpp b/Source/WebCore/editing/TextCheckingHelper.cpp
@@ -289,7 +289,7 @@ auto TextCheckingHelper::findFirstMisspelledWordOrUngrammaticalPhrase(bool check
     if (!unifiedTextCheckerEnabled())
         return { };
 
-    if (platformDrivenTextCheckerEnabled())
+    if (platformOrClientDrivenTextCheckerEnabled())
         return { };
 
     std::variant<MisspelledWord, UngrammaticalPhrase> firstFoundItem;
@@ -499,7 +499,7 @@ TextCheckingGuesses TextCheckingHelper::guessesForMisspelledWordOrUngrammaticalP
     if (!unifiedTextCheckerEnabled())
         return { };
 
-    if (platformDrivenTextCheckerEnabled())
+    if (platformOrClientDrivenTextCheckerEnabled())
         return { };
 
     if (m_range.collapsed())
@@ -610,4 +610,13 @@ bool platformDrivenTextCheckerEnabled()
 #endif
 }
 
+bool platformOrClientDrivenTextCheckerEnabled()
+{
+#if ENABLE(ACCESSIBILITY) && PLATFORM(MAC)
+    if (!AXObjectCache::shouldSpellCheck())
+        return true;
+#endif
+    return platformDrivenTextCheckerEnabled();
+}
+
 }
diff --git a/Source/WebCore/editing/TextCheckingHelper.h b/Source/WebCore/editing/TextCheckingHelper.h
@@ -116,5 +116,6 @@ void checkTextOfParagraph(TextCheckerClient&, StringView, OptionSet<TextChecking
 
 bool unifiedTextCheckerEnabled(const LocalFrame*);
 bool platformDrivenTextCheckerEnabled();
+bool platformOrClientDrivenTextCheckerEnabled();
 
 } // namespace WebCore
diff --git a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h b/Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h
@@ -90,6 +90,8 @@ class AccessibilityController : public JSWrappable {
 
     void resetToConsistentState();
 
+    void overrideClient(JSStringRef clientType);
+
 #if !ENABLE(ACCESSIBILITY) && (PLATFORM(GTK) || PLATFORM(WPE))
     RefPtr<AccessibilityUIElement> rootElement() { return nullptr; }
     RefPtr<AccessibilityUIElement> focusedElement() { return nullptr; }

diff --git a/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityController.idl b/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityController.idl
@@ -48,5 +48,7 @@
     undefined logScrollingStartEvents();
     undefined logAccessibilityEvents();
     undefined resetToConsistentState();
+
+    undefined overrideClient(DOMString clientType);
 };
 
diff --git a/Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityControllerAtspi.cpp b/Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityControllerAtspi.cpp
@@ -120,6 +120,10 @@ bool AccessibilityController::removeNotificationListener()
     return true;
 }
 
+void AccessibilityController::overrideClient(JSStringRef)
+{
+}
+
 } // namespace WTR
 
 #endif // USE(ATSPI)
diff --git a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityControllerIOS.mm b/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityControllerIOS.mm
@@ -111,4 +111,8 @@ static id findAccessibleObjectById(id obj, NSString *idAttribute)
     return nullptr;
 }
 
+void AccessibilityController::overrideClient(JSStringRef)
+{
+}
+
 } // namespace WTR
diff --git a/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityControllerMac.mm b/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityControllerMac.mm
@@ -41,9 +41,10 @@
 #import <WebKit/WKBundlePage.h>
 #import <WebKit/WKBundlePagePrivate.h>
 
+#import <pal/spi/mac/HIServicesSPI.h>
+
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
 #import <pal/spi/cocoa/AccessibilitySupportSPI.h>
-#import <pal/spi/mac/HIServicesSPI.h>
 #endif
 
 namespace WTR {
@@ -154,6 +155,15 @@ static id findAccessibleObjectById(id obj, NSString *idAttribute)
 }
 #endif
 
+void AccessibilityController::overrideClient(JSStringRef clientType)
+{
+    NSString *clientString = [NSString stringWithJSStringRef:clientType];
+    if ([clientString caseInsensitiveCompare:@"voiceover"] == NSOrderedSame)
+        _AXSetClientIdentificationOverride(kAXClientTypeVoiceOver);
+    else
+        _AXSetClientIdentificationOverride(kAXClientTypeNoActiveRequestFound);
+}
+
 // AXThread implementation
 
 void AXThread::initializeRunLoop()

diff --git a/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp b/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityControllerWin.cpp
@@ -79,6 +79,10 @@ bool AccessibilityController::removeNotificationListener()
     return false;
 }
 
+void AccessibilityController::overrideClient(JSStringRef)
+{
+}
+
 } // namespace WTR
 
 #endif // ENABLE(ACCESSIBILITY)