Changeset 247098 in webkit


Ignore:
Timestamp:
Jul 3, 2019 12:53:01 PM (5 years ago)
Author:
Alan Bujtas
Message:

Source/WebKit:
[ContentChangeObserver] REGRESSION (r244356): Drop down menus collapse without user input - Ebay.com
​https://bugs.webkit.org/show_bug.cgi?id=199457
<rdar://problem/52386563>

Reviewed by Simon Fraser.

There's a fixed, 32ms window for observing content changes after the tap is committed. r244356 introduced the fast-click behavior on form elements by omitting this fixed window and
dispatch the synthetic click on the target node.
This patch preserves the fast-click behavior, but now we stay at hover if the mouseMove event triggers a synchronous actionable visiblity change (as opposed to always proceed with click).

  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::handleSyntheticClick):

LayoutTests:
REGRESSION (r244356): Drop down menus collapse without user input - Ebay.com
​https://bugs.webkit.org/show_bug.cgi?id=199457
<rdar://problem/52386563>

Reviewed by Simon Fraser.

  • fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change-expected.txt: Added.
  • fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change.html: Copied from LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html.
  • fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change-expected.txt: Added.
  • fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change.html: Copied from LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html.
  • fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html:
Location:
trunk
Files:
2 added
6 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r247093 r247098  
     12019-07-03  Zalan Bujtas  <zalan@apple.com>
     2
     3        REGRESSION (r244356): Drop down menus collapse without user input - Ebay.com
     4        https://bugs.webkit.org/show_bug.cgi?id=199457
     5        <rdar://problem/52386563>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change-expected.txt: Added.
     10        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change.html: Copied from LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html.
     11        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change-expected.txt: Added.
     12        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change.html: Copied from LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html.
     13        * fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html:
     14
    1152019-07-03  Andres Gonzalez  <andresg_22@apple.com>
    216

  • trunk/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html

    r244392 r247098  
    4444}, false);
    4545
    46 becomesVisible.addEventListener("click", function( event ) {   
    47     result.innerHTML = "clicked hidden";
    48 }, false);
    49 
    5046tapthis.addEventListener("click", function( event ) {   
    5147    result.innerHTML = "clicked";

  • trunk/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-async-clickable-change.html

    r247096 r247098  
    22<html>
    33<head>
    4 <title>This tests the case when the tap target node is a form control element.</title>
     4<title>This tests the case when the tap target node is a form control element but it triggers async actionable visibility change -> click (we want to preserve fast click behavior on form controls).</title>
    55<script src="../../../../../resources/basic-gestures.js"></script>
    66<style>
    … …  
    3939<script>
    4040tapthis.addEventListener("mousemove", function( event ) {
    41     becomesVisible.style.visibility = "visible";
     41    setTimeout(function() {
     42        becomesVisible.style.visibility = "visible";
     43    }, 100);
    4244}, false);
    4345

  • trunk/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element-with-clickable-change.html

    r247096 r247098  
    22<html>
    33<head>
    4 <title>This tests the case when the tap target node is a form control element.</title>
     4<title>This tests the case when the tap target node is a form control element but it triggers synchronous actionable visibility change -> "fast" hover.</title>
    55<script src="../../../../../resources/basic-gestures.js"></script>
    66<style>
    … …  
    3333</head>
    3434<body onload="test()">
    35 PASS if 'clicked' text is shown below.<br>
     35PASS if 'clicked' text is NOT shown below.<br>
    3636<input type="button" value="tap this button" id=tapthis>
    3737<div id=becomesVisible></div>
    … …  
    4040tapthis.addEventListener("mousemove", function( event ) {
    4141    becomesVisible.style.visibility = "visible";
     42    if (window.testRunner)
     43        setTimeout("testRunner.notifyDone()", 50);
    4244}, false);
    4345
    … …  
    4850tapthis.addEventListener("click", function( event ) {   
    4951    result.innerHTML = "clicked";
    50     if (window.testRunner)
    51         testRunner.notifyDone();
    5252}, false);
    5353</script>

  • trunk/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-button-element.html

    r244378 r247098  
    22<html>
    33<head>
    4 <title>This tests the case when the tap target node is a form control element.</title>
     4<title>This tests the case when the tap target node is a form control element -> "fast" click.</title>
    55<script src="../../../../../resources/basic-gestures.js"></script>
    66<style>
    … …  
    4242}, false);
    4343
    44 becomesVisible.addEventListener("click", function( event ) {   
    45     result.innerHTML = "clicked hidden";
    46 }, false);
    47 
    4844tapthis.addEventListener("click", function( event ) {   
    4945    result.innerHTML = "clicked";

  • trunk/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-type-text-element.html

    r244378 r247098  
    4242}, false);
    4343
    44 becomesVisible.addEventListener("click", function( event ) {   
    45     result.innerHTML = "clicked hidden";
    46 }, false);
    47 
    4844tapthis.addEventListener("click", function( event ) {   
    4945    result.innerHTML = "clicked";

  • trunk/Source/WebKit/ChangeLog

    r247096 r247098  
     12019-07-03  Zalan Bujtas  <zalan@apple.com>
     2
     3        [ContentChangeObserver] REGRESSION (r244356): Drop down menus collapse without user input - Ebay.com
     4        https://bugs.webkit.org/show_bug.cgi?id=199457
     5        <rdar://problem/52386563>
     6
     7        Reviewed by Simon Fraser.
     8
     9        There's a fixed, 32ms window for observing content changes after the tap is committed. r244356 introduced the fast-click behavior on form elements by omitting this fixed window and
     10        dispatch the synthetic click on the target node.
     11        This patch preserves the fast-click behavior, but now we stay at hover if the mouseMove event triggers a synchronous actionable visiblity change (as opposed to always proceed with click).
     12
     13        * WebProcess/WebPage/ios/WebPageIOS.mm:
     14        (WebKit::WebPage::handleSyntheticClick):
     15
    1162019-07-03  Patrick Griffis  <pgriffis@igalia.com>
    217

  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r247056 r247098  
    576576}
    577577
    578 static bool nodeAlwaysTriggersClick(const Node& targetNode)
     578static bool nodeTriggersFastPath(const Node& targetNode)
    579579{
    580580    if (!is<Element>(targetNode))
    … …  
    684684
    685685    auto observedContentChange = contentChangeObserver.observedContentChange();
    686     auto targetNodeTriggersClick = nodeAlwaysTriggersClick(nodeRespondingToClick);
    687 
    688     auto continueContentObservation = !(observedContentChange == WKContentVisibilityChange || targetNodeTriggersClick);
     686    auto targetNodeTriggersFastPath = nodeTriggersFastPath(nodeRespondingToClick);
     687
     688    auto continueContentObservation = !(observedContentChange == WKContentVisibilityChange || targetNodeTriggersFastPath);
    689689    if (continueContentObservation) {
    690690        // Wait for callback to completePendingSyntheticClickForContentChangeObserver() to decide whether to send the click event.
    … …  
    699699    }
    700700
    701     callOnMainThread([protectedThis = makeRefPtr(this), targetNode = Ref<Node>(nodeRespondingToClick), location, modifiers, observedContentChange, targetNodeTriggersClick, pointerId] {
     701    callOnMainThread([protectedThis = makeRefPtr(this), targetNode = Ref<Node>(nodeRespondingToClick), location, modifiers, observedContentChange, pointerId] {
    702702        if (protectedThis->m_isClosed || !protectedThis->corePage())
    703703            return;
    704704
    705         auto shouldStayAtHoverState = observedContentChange == WKContentVisibilityChange && !targetNodeTriggersClick;
     705        auto shouldStayAtHoverState = observedContentChange == WKContentVisibilityChange;
    706706        if (shouldStayAtHoverState) {
    707707            // The move event caused new contents to appear. Don't send synthetic click event, but just ensure that the mouse is on the most recent content.
Note: See TracChangeset for help on using the changeset viewer.