Changeset 247100 in webkit


Ignore:
Timestamp:
Jul 3, 2019 12:59:02 PM (5 years ago)
Author:
Wenson Hsieh
Message:

REGRESSION (iOS 13): Tapping an element with a click event handler no longer clears the selection
​https://bugs.webkit.org/show_bug.cgi?id=199430

Reviewed by Tim Horton.

Source/WebCore:

After <trac.webkit.org/r245067>, we no longer immediately clear the text selection when recognizing a single tap
in WKContentView, and instead only clear it out in the case where the single tap didn't result in a click event
in the web process. This fixed an issue wherein the text selection would be prematurely cleared when tapping,
but also made it such that tapping on an element with a click event handler would not cause the selection to
change, even if preventDefault() is not called on mousedown. On web pages that add a click event listener to
document.body, it's nearly impossible to dismiss text selections by tapping elsewhere in the body.

On macOS, this works because EventHandler::handleMousePressEventSingleClick contains logic to modify the
selection when handling a mousedown, as a part of default behavior. However, there is platform-specific logic
added in <trac.webkit.org/r233311> that avoids changing the selection when handling a synthetic mousedown on
iOS; this is because we defer to the single tap text interaction gesture on iOS, which (among other things)
provides additional support for moving the selection to word boundaries, instead of the editing position
directly under the click.

However, no such platform-specific text interaction single tap gesture exists for non-editable text, so there's
no reason we need to bail in the case where the root editable element is null. We can fix this bug without
breaking the fix in r233311 by matching macOS behavior and not bailing via early return in the case where the
single tap would move selection into non-editable text.

Tests: editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html

editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html

  • page/EventHandler.cpp:

(WebCore::EventHandler::handleMousePressEventSingleClick):

LayoutTests:

Add and adjust layout tests to verify that calling preventDefault() on mousedown on iOS causes an existing
selection to not be cleared, and that tapping in an element with a click handler clears out the selection.

  • editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt: Added.
  • editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html: Added.
  • editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt: Renamed.
  • editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html:

Renamed from LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html,
and adjusted to call preventDefault() on mousedown events instead of click events. Also, remove a bit of
trailing whitespace.

Location:
trunk
Files:
2 added
3 edited
2 moved

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r247099 r247100  
     12019-07-03  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (iOS 13): Tapping an element with a click event handler no longer clears the selection
     4        https://bugs.webkit.org/show_bug.cgi?id=199430
     5
     6        Reviewed by Tim Horton.
     7
     8        Add and adjust layout tests to verify that calling preventDefault() on mousedown on iOS causes an existing
     9        selection to not be cleared, and that tapping in an element with a click handler clears out the selection.
     10
     11        * editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt: Added.
     12        * editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html: Added.
     13        * editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt: Renamed.
     14        * editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html:
     15
     16        Renamed from LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html,
     17        and adjusted to call preventDefault() on mousedown events instead of click events. Also, remove a bit of
     18        trailing whitespace.
     19
    1202019-07-03  Russell Epstein  <russell_e@apple.com>
    221

  • trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt

    r247099 r247100  
    11WebKit
    22The selected text is: "WebKit"
    3 This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the click event.
     3This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the mousedown event.

  • trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html

    r247099 r247100  
    4040        var clickTarget = document.getElementById("clickTarget");
    4141
    42         clickTarget.addEventListener("click", event => {
     42        clickTarget.addEventListener("mousedown", event => {
    4343            event.preventDefault();
    4444            setTimeout(() => testRunner.notifyDone(), 0);
    4545        });
    4646
    47         var target = document.getElementById("target");       
     47        var target = document.getElementById("target");
    4848        window.getSelection().setBaseAndExtent(target, 0, target, 6);
    4949
    … …  
    5656    <div id="clickTarget"></div>
    5757    <pre>The selected text is: "<span id="result"></span>"</pre>
    58     <p>This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the click event.</p>
     58    <p>This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the mousedown event.</p>
    5959</body>
    6060</html>

  • trunk/Source/WebCore/ChangeLog

    r247095 r247100  
     12019-07-03  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (iOS 13): Tapping an element with a click event handler no longer clears the selection
     4        https://bugs.webkit.org/show_bug.cgi?id=199430
     5
     6        Reviewed by Tim Horton.
     7
     8        After <trac.webkit.org/r245067>, we no longer immediately clear the text selection when recognizing a single tap
     9        in WKContentView, and instead only clear it out in the case where the single tap didn't result in a click event
     10        in the web process. This fixed an issue wherein the text selection would be prematurely cleared when tapping,
     11        but also made it such that tapping on an element with a click event handler would not cause the selection to
     12        change, even if preventDefault() is not called on mousedown. On web pages that add a click event listener to
     13        `document.body`, it's nearly impossible to dismiss text selections by tapping elsewhere in the body.
     14
     15        On macOS, this works because EventHandler::handleMousePressEventSingleClick contains logic to modify the
     16        selection when handling a mousedown, as a part of default behavior. However, there is platform-specific logic
     17        added in <trac.webkit.org/r233311> that avoids changing the selection when handling a synthetic mousedown on
     18        iOS; this is because we defer to the single tap text interaction gesture on iOS, which (among other things)
     19        provides additional support for moving the selection to word boundaries, instead of the editing position
     20        directly under the click.
     21
     22        However, no such platform-specific text interaction single tap gesture exists for non-editable text, so there's
     23        no reason we need to bail in the case where the root editable element is null. We can fix this bug without
     24        breaking the fix in r233311 by matching macOS behavior and not bailing via early return in the case where the
     25        single tap would move selection into non-editable text.
     26
     27        Tests: editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html
     28               editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html
     29
     30        * page/EventHandler.cpp:
     31        (WebCore::EventHandler::handleMousePressEventSingleClick):
     32
    1332019-07-03  Ryan Haddad  <ryanhaddad@apple.com>
    234

  • trunk/Source/WebCore/page/EventHandler.cpp

    r247024 r247100  
    693693#if PLATFORM(IOS_FAMILY)
    694694    // The text selection assistant will handle selection in the case where we are already editing the node
    695     if (newSelection.rootEditableElement() == targetNode->rootEditableElement())
     695    auto* editableRoot = newSelection.rootEditableElement();
     696    if (editableRoot && editableRoot == targetNode->rootEditableElement())
    696697        return true;
    697698#endif
Note: See TracChangeset for help on using the changeset viewer.