<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[246096] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/246096">246096</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2019-06-04 21:15:21 -0700 (Tue, 04 Jun 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>[ContentChangeObserver] Gmail text editing controls require two taps
https://bugs.webkit.org/show_bug.cgi?id=198541
<rdar://problem/51375055>

Reviewed by Simon Fraser.

Source/WebCore:

When the animation completes we should also check if the newly visible content is also clickable and report it accordingly.
When the animated content is not clickable, we need to proceed with click instead of stopping at hover.

Test: fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html

* page/ios/ContentChangeObserver.cpp:
(WebCore::isConsideredClickable):
(WebCore::ContentChangeObserver::didFinishTransition):
(WebCore::ContentChangeObserver::adjustObservedState):
(WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
(WebCore::ContentChangeObserver::StyleChangeScope::isConsideredClickable const): Deleted. -> Turn it into a static function so that didFinishTransition could call it as well.
* page/ios/ContentChangeObserver.h:

LayoutTests:

* fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html: Added.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepageiosContentChangeObservercpp">trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp</a></li>
<li><a href="#trunkSourceWebCorepageiosContentChangeObserverh">trunk/Source/WebCore/page/ios/ContentChangeObserver.h</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasteventstouchioscontentobservation100msdelay10mstransitiononmousemovenoclickableexpectedtxt">trunk/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfasteventstouchioscontentobservation100msdelay10mstransitiononmousemovenoclickablehtml">trunk/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (246095 => 246096)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2019-06-05 03:14:20 UTC (rev 246095)
+++ trunk/LayoutTests/ChangeLog 2019-06-05 04:15:21 UTC (rev 246096)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2019-06-04  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Gmail text editing controls require two taps
+        https://bugs.webkit.org/show_bug.cgi?id=198541
+        <rdar://problem/51375055>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html: Added.
+
</ins><span class="cx"> 2019-06-04  Youenn Fablet  <youenn@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Layout test landed flaky in 245873 [ Release ] http/wpt/service-workers/service-worker-networkprocess-crash.html is a flaky failure
</span></span></pre></div>
<a id="trunkLayoutTestsfasteventstouchioscontentobservation100msdelay10mstransitiononmousemovenoclickableexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt (0 => 246096)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt                           (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt      2019-06-05 04:15:21 UTC (rev 246096)
</span><span class="lines">@@ -0,0 +1,2 @@
</span><ins>+PASS if 'clicked' text is shown below.
+clicked
</ins></span></pre></div>
<a id="trunkLayoutTestsfasteventstouchioscontentobservation100msdelay10mstransitiononmousemovenoclickablehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html (0 => 246096)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html                           (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html      2019-06-05 04:15:21 UTC (rev 246096)
</span><span class="lines">@@ -0,0 +1,57 @@
</span><ins>+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<title>This tests the case when mousemove triggers a 10ms transition with delay and the new content is not "clickable"</title>
+<script src="../../../../../resources/basic-gestures.js"></script>
+<style>
+#tapthis {
+    width: 400px;
+    height: 400px;
+    border: 1px solid green;
+}
+
+#becomesVisible {
+       position: absolute;
+       top: 100px;
+       left: -1000px;
+       width: 100px;
+       height: 100px;
+       background-color: green;
+       transition: left 10ms ease-in-out 100ms;
+}
+</style>
+<script>
+async function test() {
+    if (!window.testRunner || !testRunner.runUIScript)
+        return;
+    if (window.internals)
+        internals.settings.setContentChangeObserverEnabled(true);
+
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+
+    let rect = tapthis.getBoundingClientRect();
+    let x = rect.left + rect.width / 2;
+    let y = rect.top + rect.height / 2;
+
+    await tapAtPoint(x, y);
+}
+</script>
+</head>
+<body onload="test()">
+<div id=tapthis>PASS if 'clicked' text is shown below.</div>
+<div id=becomesVisible></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mousemove", function( event ) {
+    becomesVisible.style.left = "10px";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, false);
+</script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (246095 => 246096)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2019-06-05 03:14:20 UTC (rev 246095)
+++ trunk/Source/WebCore/ChangeLog      2019-06-05 04:15:21 UTC (rev 246096)
</span><span class="lines">@@ -1,3 +1,24 @@
</span><ins>+2019-06-04  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Gmail text editing controls require two taps
+        https://bugs.webkit.org/show_bug.cgi?id=198541
+        <rdar://problem/51375055>
+
+        Reviewed by Simon Fraser.
+
+        When the animation completes we should also check if the newly visible content is also clickable and report it accordingly.
+        When the animated content is not clickable, we need to proceed with click instead of stopping at hover.
+
+        Test: fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::isConsideredClickable):
+        (WebCore::ContentChangeObserver::didFinishTransition):
+        (WebCore::ContentChangeObserver::adjustObservedState):
+        (WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
+        (WebCore::ContentChangeObserver::StyleChangeScope::isConsideredClickable const): Deleted. -> Turn it into a static function so that didFinishTransition could call it as well.
+        * page/ios/ContentChangeObserver.h:
+
</ins><span class="cx"> 2019-06-04  Michael Catanzaro  <mcatanzaro@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         Fix miscellaneous build warnings
</span></span></pre></div>
<a id="trunkSourceWebCorepageiosContentChangeObservercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (246095 => 246096)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp  2019-06-05 03:14:20 UTC (rev 246095)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp     2019-06-05 04:15:21 UTC (rev 246096)
</span><span class="lines">@@ -83,6 +83,33 @@
</span><span class="cx">     return false;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+enum class ElementHadRenderer { No, Yes };
+static bool isConsideredClickable(const Element& newlyVisibleElement, ElementHadRenderer hadRenderer)
+{
+    auto& element = const_cast<Element&>(newlyVisibleElement);
+    if (element.isInUserAgentShadowTree())
+        return false;
+
+    if (is<HTMLIFrameElement>(element))
+        return true;
+
+    if (is<HTMLImageElement>(element)) {
+        // This is required to avoid HTMLImageElement's touch callout override logic. See rdar://problem/48937767.
+        return element.Element::willRespondToMouseClickEvents();
+    }
+
+    auto willRespondToMouseClickEvents = element.willRespondToMouseClickEvents();
+    if (hadRenderer == ElementHadRenderer::No || willRespondToMouseClickEvents)
+        return willRespondToMouseClickEvents;
+    // In case when the visible content already had renderers it's not sufficient to check the "newly visible" element only since it might just be the container for the clickable content.  
+    for (auto& descendant : descendantsOfType<RenderElement>(*element.renderer())) {
+        if (!descendant.element())
+            continue;
+        if (descendant.element()->willRespondToMouseClickEvents())
+            return true;
+    }
+    return false;
+}
</ins><span class="cx"> ContentChangeObserver::ContentChangeObserver(Document& document)
</span><span class="cx">     : m_document(document)
</span><span class="cx">     , m_contentObservationTimer([this] { completeDurationBasedContentObservation(); })
</span><span class="lines">@@ -159,7 +186,11 @@
</span><span class="cx">         return;
</span><span class="cx">     LOG_WITH_STREAM(ContentObservation, stream << "didFinishTransition: transition finished (" << &element << ").");
</span><span class="cx"> 
</span><del>-    adjustObservedState(isConsideredHidden(element) ? Event::EndedTransition : Event::CompletedTransition);
</del><ins>+    if (isConsideredHidden(element)) {
+        adjustObservedState(Event::EndedTransitionButFinalStyleIsNotDefiniteYet);
+        return;
+    }
+    adjustObservedState(isConsideredClickable(element, ElementHadRenderer::Yes) ? Event::CompletedTransitionWithClickableContent : Event::CompletedTransitionWithoutClickableContent);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void ContentChangeObserver::didRemoveTransition(const Element& element, CSSPropertyID propertyID)
</span><span class="lines">@@ -472,7 +503,7 @@
</span><span class="cx">         if (!isObservationTimeWindowActive())
</span><span class="cx">             adjustStateAndNotifyContentChangeIfNeeded();
</span><span class="cx">         break;
</span><del>-    case Event::EndedTransition:
</del><ins>+    case Event::EndedTransitionButFinalStyleIsNotDefiniteYet:
</ins><span class="cx">         // onAnimationEnd can be called while in the middle of resolving the document (synchronously) or
</span><span class="cx">         // asynchronously right before the style update is issued. It also means we don't know whether this animation ends up producing visible content yet. 
</span><span class="cx">         if (m_document.inStyleRecalc()) {
</span><span class="lines">@@ -481,9 +512,11 @@
</span><span class="cx">         } else
</span><span class="cx">             setShouldObserveNextStyleRecalc(true);
</span><span class="cx">         break;
</span><del>-    case Event::CompletedTransition:
</del><ins>+    case Event::CompletedTransitionWithClickableContent:
</ins><span class="cx">         // Set visibility flag on and report visible change synchronously or asynchronously depending whether we are in the middle of style recalc.
</span><span class="cx">         contentVisibilityDidChange();
</span><ins>+        FALLTHROUGH;
+    case Event::CompletedTransitionWithoutClickableContent:
</ins><span class="cx">         if (m_document.inStyleRecalc())
</span><span class="cx">             m_isInObservedStyleRecalc = true;
</span><span class="cx">         else if (!isObservationTimeWindowActive())
</span><span class="lines">@@ -520,38 +553,10 @@
</span><span class="cx">         return m_wasHidden && !isConsideredHidden(m_element);
</span><span class="cx">     };
</span><span class="cx"> 
</span><del>-    if (changedFromHiddenToVisible() && isConsideredClickable())
</del><ins>+    if (changedFromHiddenToVisible() && isConsideredClickable(m_element, m_hadRenderer ? ElementHadRenderer::Yes : ElementHadRenderer::No))
</ins><span class="cx">         m_contentChangeObserver.contentVisibilityDidChange();
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-bool ContentChangeObserver::StyleChangeScope::isConsideredClickable() const
-{
-    if (m_element.isInUserAgentShadowTree())
-        return false;
-
-    auto& element = const_cast<Element&>(m_element);
-    if (is<HTMLIFrameElement>(element))
-        return true;
-
-    if (is<HTMLImageElement>(element)) {
-        // This is required to avoid HTMLImageElement's touch callout override logic. See rdar://problem/48937767.
-        return element.Element::willRespondToMouseClickEvents();
-    }
-
-    auto willRespondToMouseClickEvents = element.willRespondToMouseClickEvents();
-    if (!m_hadRenderer || willRespondToMouseClickEvents)
-        return willRespondToMouseClickEvents;
-    // In case when the visible content already had renderers it's not sufficient to check the "newly visible" element only since it might just be the container for the clickable content.  
-    ASSERT(m_element.renderer());
-    for (auto& descendant : descendantsOfType<RenderElement>(*element.renderer())) {
-        if (!descendant.element())
-            continue;
-        if (descendant.element()->willRespondToMouseClickEvents())
-            return true;
-    }
-    return false;
-}
-
</del><span class="cx"> #if ENABLE(TOUCH_EVENTS)
</span><span class="cx"> ContentChangeObserver::TouchEventScope::TouchEventScope(Document& document, PlatformEvent::Type eventType)
</span><span class="cx">     : m_contentChangeObserver(document.contentChangeObserver())
</span></span></pre></div>
<a id="trunkSourceWebCorepageiosContentChangeObserverh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.h (246095 => 246096)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/ios/ContentChangeObserver.h    2019-06-05 03:14:20 UTC (rev 246095)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.h       2019-06-05 04:15:21 UTC (rev 246096)
</span><span class="lines">@@ -71,8 +71,6 @@
</span><span class="cx">         ~StyleChangeScope();
</span><span class="cx"> 
</span><span class="cx">     private:
</span><del>-        bool isConsideredClickable() const;
-
</del><span class="cx">         ContentChangeObserver& m_contentChangeObserver;
</span><span class="cx">         const Element& m_element;
</span><span class="cx">         bool m_wasHidden { false };
</span><span class="lines">@@ -189,8 +187,9 @@
</span><span class="cx">         StartedStyleRecalc,
</span><span class="cx">         EndedStyleRecalc,
</span><span class="cx">         AddedTransition,
</span><del>-        EndedTransition,
-        CompletedTransition,
</del><ins>+        EndedTransitionButFinalStyleIsNotDefiniteYet,
+        CompletedTransitionWithClickableContent,
+        CompletedTransitionWithoutClickableContent,
</ins><span class="cx">         CanceledTransition,
</span><span class="cx">         StartedFixedObservationTimeWindow,
</span><span class="cx">         EndedFixedObservationTimeWindow,
</span></span></pre>
</div>
</div>

</body>
</html>