Attachment 369690 Details for Bug 197813 – Patch

[patch] Patch

bug-197813-20190512205123.patch (text/plain), 6.34 KB, created by Wenson Hsieh on 2019-05-12 20:51:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-05-12 20:51:23 PDT

Size: 6.34 KB

patch

obsolete

>Subversion Revision: 245205
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d53874bd5352c7cd85e001c86d4db79bc421a931..5ff42c326a061287cd27e75b16d02d4c05e68959 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-05-12  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [macOS] Font formatting options don't work when composing a message in Yahoo mail
>+        https://bugs.webkit.org/show_bug.cgi?id=197813
>+        <rdar://problem/49382250>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The bug happens because on mousedown, the "Aa Font" menu item's event handler hides itself before changing the
>+        font at the text selection. This causes us to clear the selection in FocusController::setFocusedElement.
>+
>+        There is existing logic in clearSelectionIfNeeded that would normally prevent us from clearing the selection due
>+        to the mousePressNode not being able to start a selection. However, since the clickable element in this case is
>+        hidden during mousedown, it is missing a renderer, and we bail from the `mousePressNode->renderer() &&
>+        !mousePressNode->canStartSelection()` check as a result.
>+
>+        This check was orginally added in https://trac.webkit.org/r24334 to avoid clearing the selection when clicking
>+        a button; the intention appears to have been making it so that clicking on something that could not start a
>+        selection (back then, synonymous with -webkit-user-select: ignore;) would not clear the current selection; to
>+        this end, it seems odd to additionally require that the thing being clicked should still have a renderer, so
>+        it seems safe to remove this requirement.
>+
>+        Test: editing/selection/preserve-selection-when-clicking-button.html
>+
>+        * page/FocusController.cpp:
>+        (WebCore::clearSelectionIfNeeded):
>+
> 2019-05-11  Simon Fraser  <simon.fraser@apple.com>
> 
>         Backing-sharing layers with transforms render incorrectly
>diff --git a/Source/WebCore/page/FocusController.cpp b/Source/WebCore/page/FocusController.cpp
>index 75276b237556d3d639c181196df26843208a8372..d29c78f7a38bc9c0cd21aaea753c03182826fa32 100644
>--- a/Source/WebCore/page/FocusController.cpp
>+++ b/Source/WebCore/page/FocusController.cpp
>@@ -774,7 +774,7 @@ static void clearSelectionIfNeeded(Frame* oldFocusedFrame, Frame* newFocusedFram
>     }
> 
>     if (Node* mousePressNode = newFocusedFrame->eventHandler().mousePressNode()) {
>-        if (mousePressNode->renderer() && !mousePressNode->canStartSelection()) {
>+        if (!mousePressNode->canStartSelection()) {
>             // Don't clear the selection for contentEditable elements, but do clear it for input and textarea. See bug 38696.
>             auto* root = selection.rootEditableElement();
>             if (!root)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index cfb54ec997b3dd6aad439fff10d08c9023f86465..1ad39ba5a4a564b6c51658b142db85f14354be80 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-12  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [macOS] Font formatting options don't work when composing a message in Yahoo mail
>+        https://bugs.webkit.org/show_bug.cgi?id=197813
>+        <rdar://problem/49382250>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a new layout test to verify that DOM selection is preserved after clicking a button that hides itself
>+        upon mousedown.
>+
>+        * editing/selection/preserve-selection-when-clicking-button-expected.txt: Added.
>+        * editing/selection/preserve-selection-when-clicking-button.html: Added.
>+
> 2019-05-11  Simon Fraser  <simon.fraser@apple.com>
> 
>         Backing-sharing layers with transforms render incorrectly
>diff --git a/LayoutTests/editing/selection/preserve-selection-when-clicking-button-expected.txt b/LayoutTests/editing/selection/preserve-selection-when-clicking-button-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..14de6e51362d0edd47b900dd19df6d110123d108
>--- /dev/null
>+++ b/LayoutTests/editing/selection/preserve-selection-when-clicking-button-expected.txt
>@@ -0,0 +1,3 @@
>+Hello world
>+The selected text after clicking is: "Hello world"
>+This test verifies that clicking a button which hides itself on mousedown does not clear the selection. To test manually, select "Hello world" above and click the button. The pre element above should indicate that "Hello world" was selected after clicking.
>diff --git a/LayoutTests/editing/selection/preserve-selection-when-clicking-button.html b/LayoutTests/editing/selection/preserve-selection-when-clicking-button.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..8305b7260b86291806b7f757e0b5715f89b4f4f2
>--- /dev/null
>+++ b/LayoutTests/editing/selection/preserve-selection-when-clicking-button.html
>@@ -0,0 +1,45 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <meta name="viewport" content="width=device-width, initial-scale=1">
>+    <script src="../../resources/ui-helper.js"></script>
>+</head>
>+<body>
>+    <div class="editor" contenteditable>Hello world</div>
>+    <pre>The selected text after clicking is: "<span class="output"></span>"</pre>
>+    <div>
>+        <button>Click here</button>
>+    </div>
>+    <p>This test verifies that clicking a button which hides itself on mousedown does not clear the selection. To test manually, select "Hello world" above and click the button. The <code>pre</code> element above should indicate that "Hello world" was selected after clicking.</p>
>+    <script>
>+    if (window.testRunner) {
>+        testRunner.waitUntilDone();
>+        testRunner.dumpAsText();
>+    }
>+
>+    const editor = document.querySelector(".editor");
>+    const button = document.querySelector("button");
>+    const output = document.querySelector(".output");
>+
>+    function checkDone() {
>+        doneCount = window.doneCount ? doneCount : 0;
>+        if (++doneCount == 2 && window.testRunner)
>+            testRunner.notifyDone();
>+    }
>+
>+    getSelection().selectAllChildren(editor);
>+    button.addEventListener("mousedown", () => {
>+        button.style.display = "none";
>+        setTimeout(() => {
>+            output.textContent = getSelection().toString();
>+            checkDone();
>+        });
>+    });
>+
>+    addEventListener("load", () => {
>+        if (window.testRunner)
>+            UIHelper.activateElement(button).then(checkDone);
>+    });
>+    </script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 197813: 369690