Unable to show text alternatives when tapping grammatical error in Ma…

…il compose https://bugs.webkit.org/show_bug.cgi?id=259008 rdar://111404885 Reviewed by Tim Horton and Abrar Rahman Protyasha. After the changes in 264745@main, we no longer perform sync IPC underneath `-requestAutocorrectionContextWithCompletionHandler:`, and instead just return an empty context when out-of-process keyboard is enabled. This is because UIKit now syncs document state by requesting `UIWKDocumentContext`s when OOPK is enabled, instead of `UIWKAutocorrectionContext`. However, there's still one instance where UIKit still depends on the legacy autocorrection context to drive platform features: the case of showing grammar text checking results when the user taps on a grammar correction in editable content (i.e. words with blue dotted underlines). Work around this for the time being by falling back to shipping behavior when grammar corrections are present in the document; we can remove this once rdar://111936621 is fixed. * LayoutTests/editing/selection/ios/show-grammar-replacements-on-tap-expected.txt: Added. * LayoutTests/editing/selection/ios/show-grammar-replacements-on-tap.html: Added. * LayoutTests/platform/ios-wk2/TestExpectations: Add a layout test to exercise the fix, building on the infrastructure added in 265869@main. Note that we use `runSingly=true` here because otherwise, `UIWKTextInteractionAssistant` might use a previously initialized and cached `UITextChecker` instance, rather than the `LayoutTestSpellChecker` that contains the canned text corrections that the test depends on. * Source/WebCore/dom/DocumentMarkerController.h: * Source/WebKit/Shared/EditorState.h: * Source/WebKit/Shared/EditorState.serialization.in: Add a new bit to `EditorState` here, indicating whether we have grammar markers inside the current editable root. * Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm: (-[WKContentView requestAutocorrectionContextWithCompletionHandler:]): * Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm: (WebKit::WebPage::getPlatformEditorState const): Canonical link: https://commits.webkit.org/265880@main
WebKit · Jul 8, 2023 · e4ff4c6 · e4ff4c6
1 parent ea22468
commit e4ff4c6
Show file tree

Hide file tree

Showing 8 changed files with 103 additions and 6 deletions.
diff --git a/LayoutTests/editing/selection/ios/show-grammar-replacements-on-tap-expected.txt b/LayoutTests/editing/selection/ios/show-grammar-replacements-on-tap-expected.txt
@@ -0,0 +1,14 @@
+This test verifies that tapping on an editable grammar text checking range shows the edit menu and selects the range encompassing the grammatical error. This test requires WebKitTestRunner to run.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Presented edit menu
+PASS getSelection().toString() is "thing"
+PASS internals.hasGrammarMarker(0, 5) is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+I'll need to thing about it.
+
+
diff --git a/LayoutTests/editing/selection/ios/show-grammar-replacements-on-tap.html b/LayoutTests/editing/selection/ios/show-grammar-replacements-on-tap.html
@@ -0,0 +1,69 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true runSingly=true ] -->
+<html>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+<style>
+div[contenteditable] {
+    font-family: monospace;
+    font-size: 32px;
+    border: 1px solid tomato;
+    width: 100%;
+}
+</style>
+<script>
+jsTestIsAsync = true;
+description(`This test verifies that tapping on an editable grammar text checking range shows the
+    edit menu and selects the range encompassing the grammatical error. This test requires
+    WebKitTestRunner to run.`);
+
+addEventListener("load", async () => {
+    const results = [
+        {
+            "type": "grammar",
+            "from": 13,
+            "to": 18,
+            "details": [{ "from": 0, "to": 5, "corrections": ["think"] }],
+        }
+    ];
+    await UIHelper.setSpellCheckerResults({
+        "I'll need to thing about it.\n": results,
+        "I'll need to thing about it.": results
+    });
+
+    let target = document.getElementById("target");
+    let editor = document.querySelector("div[contenteditable]");
+
+    await UIHelper.activateElementAndWaitForInputSession(editor);
+
+    // First, trigger grammar checking by inserting a newline.
+    getSelection().selectAllChildren(editor);
+    getSelection().collapseToEnd();
+    document.execCommand("InsertText", true, ".");
+    document.execCommand("InsertParagraph", true);
+    await UIHelper.ensurePresentationUpdate();
+
+    // Then, tap the grammar error and wait for the edit menu to show up.
+    await UIHelper.activateElement(target);
+    await UIHelper.waitForMenuToShow();
+
+    testPassed("Presented edit menu");
+    shouldBeEqualToString("getSelection().toString()", "thing");
+    shouldBeTrue("internals.hasGrammarMarker(0, 5)");
+
+    editor.blur();
+    await UIHelper.waitForKeyboardToHide();
+
+    finishJSTest();
+});
+</script>
+</head>
+<body>
+<div contenteditable>
+I'll need to <span id="target">thing</span> about it
+</div>
+<pre id="description"></pre>
+<pre id="console"></pre>
+</body>
+</html>
diff --git a/LayoutTests/platform/ios-wk2/TestExpectations b/LayoutTests/platform/ios-wk2/TestExpectations
@@ -2450,4 +2450,7 @@ webkit.org/b/257182 imported/w3c/web-platform-tests/css/css-text/white-space/tra
 
 # Layout tests that fail for iOS only for css-highlights 
 imported/w3c/web-platform-tests/css/css-highlight-api/painting/custom-highlight-painting-iframe-003.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-highlight-api/painting/custom-highlight-painting-iframe-004.html [ ImageOnlyFailure ]
+imported/w3c/web-platform-tests/css/css-highlight-api/painting/custom-highlight-painting-iframe-004.html [ ImageOnlyFailure ]
+
+# Requires grammar checking to be enabled (rdar://103646683)
+editing/selection/ios/show-grammar-replacements-on-tap.html [ Skip ]
diff --git a/Source/WebCore/dom/DocumentMarkerController.h b/Source/WebCore/dom/DocumentMarkerController.h
@@ -59,7 +59,7 @@ class DocumentMarkerController {
 
     void copyMarkers(Node& source, OffsetRange, Node& destination);
     bool hasMarkers() const;
-    bool hasMarkers(const SimpleRange&, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers());
+    WEBCORE_EXPORT bool hasMarkers(const SimpleRange&, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers());
 
     // When a marker partially overlaps with range, if removePartiallyOverlappingMarkers is true, we completely
     // remove the marker. If the argument is false, we will adjust the span of the marker so that it retains

diff --git a/Source/WebKit/Shared/EditorState.h b/Source/WebKit/Shared/EditorState.h
@@ -114,8 +114,9 @@ struct EditorState {
         bool atStartOfSentence { false };
         bool selectionStartIsAtParagraphBoundary { false };
         bool selectionEndIsAtParagraphBoundary { false };
+        bool hasGrammarDocumentMarkers { false };
         std::optional<WebCore::ElementContext> selectedEditableImage;
-#endif
+#endif // PLATFORM(IOS_FAMILY)
 #if PLATFORM(MAC)
         WebCore::IntRect selectionBoundingRect; // FIXME: Maybe this should be on VisualData?
         uint64_t candidateRequestStartPosition { 0 };

diff --git a/Source/WebKit/Shared/EditorState.serialization.in b/Source/WebKit/Shared/EditorState.serialization.in
@@ -91,6 +91,7 @@ struct WebKit::EditorState {
     bool atStartOfSentence;
     bool selectionStartIsAtParagraphBoundary;
     bool selectionEndIsAtParagraphBoundary;
+    bool hasGrammarDocumentMarkers;
     std::optional<WebCore::ElementContext> selectedEditableImage;
 #endif
 #if PLATFORM(MAC)

diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
@@ -5192,7 +5192,15 @@ - (void)requestAutocorrectionContextWithCompletionHandler:(void (^)(UIWKAutocorr
         return;
     }
 
-    if ([UIKeyboard usesInputSystemUI]) {
+    // FIXME: We can remove this code and unconditionally ignore autocorrection context requests when
+    // out-of-process keyboard is enabled, once rdar://111936621 is addressed; we'll also be able to
+    // remove `EditorState::PostLayoutData::hasGrammarDocumentMarkers` then.
+    bool requiresContextToShowGrammarCheckingResults = [&] {
+        auto& state = _page->editorState();
+        return state.hasPostLayoutData() && state.postLayoutData->hasGrammarDocumentMarkers;
+    }();
+
+    if ([UIKeyboard usesInputSystemUI] && !requiresContextToShowGrammarCheckingResults) {
         completionHandler(WKAutocorrectionContext.emptyAutocorrectionContext);
         return;
     }

diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
@@ -312,7 +312,6 @@ static void convertContentToRootView(const LocalFrameView& view, Vector<Selectio
             VisibleSelection compositionSelection(*compositionRange);
             visualData.markedTextCaretRectAtStart = view->contentsToRootView(compositionSelection.visibleStart().absoluteCaretBounds(nullptr /* insideFixed */));
             visualData.markedTextCaretRectAtEnd = view->contentsToRootView(compositionSelection.visibleEnd().absoluteCaretBounds(nullptr /* insideFixed */));
-
         }
     }
 
@@ -387,8 +386,10 @@ static void convertContentToRootView(const LocalFrameView& view, Vector<Selectio
             // FIXME: The caret color style should be computed using the selection caret's container
             // rather than the focused element. This causes caret colors in editable children to be
             // ignored in favor of the editing host's caret color. See: <https://webkit.org/b/229809>.
-            if (RefPtr editableRoot = selection.rootEditableElement(); editableRoot && editableRoot->renderer())
+            if (RefPtr editableRoot = selection.rootEditableElement(); editableRoot && editableRoot->renderer()) {
                 postLayoutData.caretColor = CaretBase::computeCaretColor(editableRoot->renderer()->style(), editableRoot.get(), std::nullopt);
+                postLayoutData.hasGrammarDocumentMarkers = editableRoot->document().markers().hasMarkers(makeRangeSelectingNodeContents(*editableRoot), DocumentMarker::Grammar);
+            }
         }
 
         computeEditableRootHasContentAndPlainText(selection, postLayoutData);