Title: [281970] trunk
Revision
281970
Author
[email protected]
Date
2021-09-02 18:17:19 -0700 (Thu, 02 Sep 2021)

Log Message

REGRESSION (r280767): Caret color is black after pasting rich text in Mail compose in dark mode
https://bugs.webkit.org/show_bug.cgi?id=229808
rdar://82600990

Reviewed by Tim Horton.

Source/WebKit:

Partially revert one of the changes in r280767 that makes caret-color follow the immediate container node of the
selection caret, rather than the focused element (which it previously used). This was effectively a drive-by fix
for a FIXME about allowing `caret-color` to match child containers inside the root editable container element.
While this matches behavior on macOS, this change also causes the caret color to become black when pasting rich
text copied in dark mode, since we write `caret-color: rgb(0, 0, 0);` to the pasteboard when copying the editing
style for rich text.

In the short term, we should restore old (pre-r280767) `caret-color` on iOS; in the slightly longer term, I've
filed bug #229809 to track unifying macOS and iOS caret-color behavior, with respect to nested editable
container nodes.

To retain the fix for rdar://81674787 without exhibiting the above bug, we use the selection's root editable
container (rather than `m_focusedElement`, which was used prior to r280767).

Test: editing/caret/ios/caret-color-in-nested-editable-containers.html

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::getPlatformEditorState const):

Also adjust this logic so that we only attempt the tree-walk to find the root editable element if the selection
has editable style.

LayoutTests:

Add a test to exercise the behavior of different `caret-color` values in nested editable elements.

* editing/caret/ios/caret-color-in-nested-editable-containers-expected.txt: Added.
* editing/caret/ios/caret-color-in-nested-editable-containers.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (281969 => 281970)


--- trunk/LayoutTests/ChangeLog	2021-09-03 01:10:03 UTC (rev 281969)
+++ trunk/LayoutTests/ChangeLog	2021-09-03 01:17:19 UTC (rev 281970)
@@ -1,3 +1,16 @@
+2021-09-02  Wenson Hsieh  <[email protected]>
+
+        REGRESSION (r280767): Caret color is black after pasting rich text in Mail compose in dark mode
+        https://bugs.webkit.org/show_bug.cgi?id=229808
+        rdar://82600990
+
+        Reviewed by Tim Horton.
+
+        Add a test to exercise the behavior of different `caret-color` values in nested editable elements.
+
+        * editing/caret/ios/caret-color-in-nested-editable-containers-expected.txt: Added.
+        * editing/caret/ios/caret-color-in-nested-editable-containers.html: Added.
+
 2021-09-02  Fujii Hironori  <[email protected]>
 
         [WinCairo] Unreviewed test gardening

Added: trunk/LayoutTests/editing/caret/ios/caret-color-in-nested-editable-containers-expected.txt (0 => 281970)


--- trunk/LayoutTests/editing/caret/ios/caret-color-in-nested-editable-containers-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/caret/ios/caret-color-in-nested-editable-containers-expected.txt	2021-09-03 01:17:19 UTC (rev 281970)
@@ -0,0 +1,11 @@
+
+This test verifies exercises the behavior of caret-color in nested editable containers. To manually run the test, focus the red box and verify that the caret color is red.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS caretColor is "rgb(255, 0, 0)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/editing/caret/ios/caret-color-in-nested-editable-containers.html (0 => 281970)


--- trunk/LayoutTests/editing/caret/ios/caret-color-in-nested-editable-containers.html	                        (rev 0)
+++ trunk/LayoutTests/editing/caret/ios/caret-color-in-nested-editable-containers.html	2021-09-03 01:17:19 UTC (rev 281970)
@@ -0,0 +1,53 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<script src=""
+<script src=""
+<style>
+body, html {
+    width: 100%;
+    height: 100%;
+    margin: 0;
+}
+
+#editor {
+    width: 250px;
+    font-size: 20px;
+    padding: 10px;
+    display: block;
+    border: 1px solid tomato;
+    caret-color: red;
+}
+</style>
+<script>
+jsTestIsAsync = true;
+
+addEventListener("load", async () => {
+    // FIXME: Once caret color follows the selection container's caret-color (bug #229809), this test should be
+    // rebaselined to verify that the caret background color is blue.
+    description("This test verifies exercises the behavior of <code>caret-color</code> in nested editable containers. To manually run the test, focus the red box and verify that the caret color is red.");
+
+    if (!window.testRunner)
+        return;
+
+    let editor = document.getElementById("editor");
+    await UIHelper.activateElementAndWaitForInputSession(editor);
+    await UIHelper.ensurePresentationUpdate();
+    caretColor = await UIHelper.selectionCaretBackgroundColor();
+
+    shouldBeEqualToString("caretColor", "rgb(255, 0, 0)");
+
+    editor.blur();
+    await UIHelper.waitForKeyboardToHide();
+    finishJSTest();
+});
+</script>
+</head>
+<body>
+<div contenteditable id="editor">
+    <div><br style="caret-color: blue;"></div>
+</div>
+<pre id="console"></pre>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebKit/ChangeLog (281969 => 281970)


--- trunk/Source/WebKit/ChangeLog	2021-09-03 01:10:03 UTC (rev 281969)
+++ trunk/Source/WebKit/ChangeLog	2021-09-03 01:17:19 UTC (rev 281970)
@@ -1,3 +1,33 @@
+2021-09-02  Wenson Hsieh  <[email protected]>
+
+        REGRESSION (r280767): Caret color is black after pasting rich text in Mail compose in dark mode
+        https://bugs.webkit.org/show_bug.cgi?id=229808
+        rdar://82600990
+
+        Reviewed by Tim Horton.
+
+        Partially revert one of the changes in r280767 that makes caret-color follow the immediate container node of the
+        selection caret, rather than the focused element (which it previously used). This was effectively a drive-by fix
+        for a FIXME about allowing `caret-color` to match child containers inside the root editable container element.
+        While this matches behavior on macOS, this change also causes the caret color to become black when pasting rich
+        text copied in dark mode, since we write `caret-color: rgb(0, 0, 0);` to the pasteboard when copying the editing
+        style for rich text.
+
+        In the short term, we should restore old (pre-r280767) `caret-color` on iOS; in the slightly longer term, I've
+        filed bug #229809 to track unifying macOS and iOS caret-color behavior, with respect to nested editable
+        container nodes.
+
+        To retain the fix for rdar://81674787 without exhibiting the above bug, we use the selection's root editable
+        container (rather than `m_focusedElement`, which was used prior to r280767).
+
+        Test: editing/caret/ios/caret-color-in-nested-editable-containers.html
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::getPlatformEditorState const):
+
+        Also adjust this logic so that we only attempt the tree-walk to find the root editable element if the selection
+        has editable style.
+
 2021-09-02  Alex Christensen  <[email protected]>
 
         Gracefully recover from WebAuthnProcess crashes

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (281969 => 281970)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-09-03 01:10:03 UTC (rev 281969)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-09-03 01:17:19 UTC (rev 281970)
@@ -336,8 +336,13 @@
     postLayoutData.atStartOfSentence = frame.selection().selectionAtSentenceStart();
     postLayoutData.insideFixedPosition = startNodeIsInsideFixedPosition || endNodeIsInsideFixedPosition;
     if (!selection.isNone()) {
-        if (RefPtr container = selection.start().containerNode(); container && container->renderer() && selection.isContentEditable())
-            postLayoutData.caretColor = CaretBase::computeCaretColor(container->renderer()->style(), container.get());
+        if (selection.hasEditableStyle()) {
+            // 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())
+                postLayoutData.caretColor = CaretBase::computeCaretColor(editableRoot->renderer()->style(), editableRoot.get());
+        }
 
         if (auto editableRootOrFormControl = makeRefPtr(enclosingTextFormControl(selection.start()) ?: selection.rootEditableElement())) {
             postLayoutData.selectionClipRect = rootViewInteractionBounds(*editableRootOrFormControl);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to