Title: [245306] branches/safari-608.1.24-branch
Revision
245306
Author
[email protected]
Date
2019-05-14 14:07:45 -0700 (Tue, 14 May 2019)

Log Message

Cherry-pick r245238. rdar://problem/49382250

    [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 Darin Adler.

    Source/WebCore:

    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):

    LayoutTests:

    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.

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245238 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-608.1.24-branch/LayoutTests/ChangeLog (245305 => 245306)


--- branches/safari-608.1.24-branch/LayoutTests/ChangeLog	2019-05-14 21:07:41 UTC (rev 245305)
+++ branches/safari-608.1.24-branch/LayoutTests/ChangeLog	2019-05-14 21:07:45 UTC (rev 245306)
@@ -1,3 +1,59 @@
+2019-05-14  Alan Coon  <[email protected]>
+
+        Cherry-pick r245238. rdar://problem/49382250
+
+    [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 Darin Adler.
+    
+    Source/WebCore:
+    
+    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):
+    
+    LayoutTests:
+    
+    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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245238 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-05-13  Wenson Hsieh  <[email protected]>
+
+            [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 Darin Adler.
+
+            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-12  Babak Shafiei  <[email protected]>
 
         Cherry-pick r245191. rdar://problem/48027412

Added: branches/safari-608.1.24-branch/LayoutTests/editing/selection/preserve-selection-when-clicking-button-expected.txt (0 => 245306)


--- branches/safari-608.1.24-branch/LayoutTests/editing/selection/preserve-selection-when-clicking-button-expected.txt	                        (rev 0)
+++ branches/safari-608.1.24-branch/LayoutTests/editing/selection/preserve-selection-when-clicking-button-expected.txt	2019-05-14 21:07:45 UTC (rev 245306)
@@ -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.

Added: branches/safari-608.1.24-branch/LayoutTests/editing/selection/preserve-selection-when-clicking-button.html (0 => 245306)


--- branches/safari-608.1.24-branch/LayoutTests/editing/selection/preserve-selection-when-clicking-button.html	                        (rev 0)
+++ branches/safari-608.1.24-branch/LayoutTests/editing/selection/preserve-selection-when-clicking-button.html	2019-05-14 21:07:45 UTC (rev 245306)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <script src=""
+</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>

Modified: branches/safari-608.1.24-branch/Source/WebCore/ChangeLog (245305 => 245306)


--- branches/safari-608.1.24-branch/Source/WebCore/ChangeLog	2019-05-14 21:07:41 UTC (rev 245305)
+++ branches/safari-608.1.24-branch/Source/WebCore/ChangeLog	2019-05-14 21:07:45 UTC (rev 245306)
@@ -1,3 +1,72 @@
+2019-05-14  Alan Coon  <[email protected]>
+
+        Cherry-pick r245238. rdar://problem/49382250
+
+    [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 Darin Adler.
+    
+    Source/WebCore:
+    
+    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):
+    
+    LayoutTests:
+    
+    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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245238 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-05-13  Wenson Hsieh  <[email protected]>
+
+            [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 Darin Adler.
+
+            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-12  Babak Shafiei  <[email protected]>
 
         Cherry-pick r245197. rdar://problem/50628434

Modified: branches/safari-608.1.24-branch/Source/WebCore/page/FocusController.cpp (245305 => 245306)


--- branches/safari-608.1.24-branch/Source/WebCore/page/FocusController.cpp	2019-05-14 21:07:41 UTC (rev 245305)
+++ branches/safari-608.1.24-branch/Source/WebCore/page/FocusController.cpp	2019-05-14 21:07:45 UTC (rev 245306)
@@ -774,7 +774,7 @@
     }
 
     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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to