Title: [246243] tags/Safari-608.1.28.1
Revision
246243
Author
[email protected]
Date
2019-06-09 14:15:50 -0700 (Sun, 09 Jun 2019)

Log Message

Cherry-pick r246096. rdar://problem/51375055

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

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

Modified Paths

Added Paths

Diff

Modified: tags/Safari-608.1.28.1/LayoutTests/ChangeLog (246242 => 246243)


--- tags/Safari-608.1.28.1/LayoutTests/ChangeLog	2019-06-09 21:06:28 UTC (rev 246242)
+++ tags/Safari-608.1.28.1/LayoutTests/ChangeLog	2019-06-09 21:15:50 UTC (rev 246243)
@@ -1,3 +1,45 @@
+2019-06-09  Babak Shafiei  <[email protected]>
+
+        Cherry-pick r246096. rdar://problem/51375055
+
+    [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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246096 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-06-04  Zalan Bujtas  <[email protected]>
+
+            [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.
+
 2019-06-04  Youenn Fablet  <[email protected]>
 
         Layout test landed flaky in 245873 [ Release ] http/wpt/service-workers/service-worker-networkprocess-crash.html is a flaky failure

Added: tags/Safari-608.1.28.1/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt (0 => 246243)


--- tags/Safari-608.1.28.1/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt	                        (rev 0)
+++ tags/Safari-608.1.28.1/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt	2019-06-09 21:15:50 UTC (rev 246243)
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is shown below.
+clicked

Added: tags/Safari-608.1.28.1/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html (0 => 246243)


--- tags/Safari-608.1.28.1/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html	                        (rev 0)
+++ tags/Safari-608.1.28.1/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html	2019-06-09 21:15:50 UTC (rev 246243)
@@ -0,0 +1,57 @@
+<!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=""
+<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>

Modified: tags/Safari-608.1.28.1/Source/WebCore/ChangeLog (246242 => 246243)


--- tags/Safari-608.1.28.1/Source/WebCore/ChangeLog	2019-06-09 21:06:28 UTC (rev 246242)
+++ tags/Safari-608.1.28.1/Source/WebCore/ChangeLog	2019-06-09 21:15:50 UTC (rev 246243)
@@ -1,3 +1,56 @@
+2019-06-09  Babak Shafiei  <[email protected]>
+
+        Cherry-pick r246096. rdar://problem/51375055
+
+    [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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246096 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-06-04  Zalan Bujtas  <[email protected]>
+
+            [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:
+
 2019-06-04  Keith Rollin  <[email protected]>
 
         Fix 64-bit vs 32-bit mismatch in ISOFairPlayStreamingPsshBox.cpp

Modified: tags/Safari-608.1.28.1/Source/WebCore/page/ios/ContentChangeObserver.cpp (246242 => 246243)


--- tags/Safari-608.1.28.1/Source/WebCore/page/ios/ContentChangeObserver.cpp	2019-06-09 21:06:28 UTC (rev 246242)
+++ tags/Safari-608.1.28.1/Source/WebCore/page/ios/ContentChangeObserver.cpp	2019-06-09 21:15:50 UTC (rev 246243)
@@ -83,6 +83,33 @@
     return false;
 }
 
+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;
+}
 ContentChangeObserver::ContentChangeObserver(Document& document)
     : m_document(document)
     , m_contentObservationTimer([this] { completeDurationBasedContentObservation(); })
@@ -159,7 +186,11 @@
         return;
     LOG_WITH_STREAM(ContentObservation, stream << "didFinishTransition: transition finished (" << &element << ").");
 
-    adjustObservedState(isConsideredHidden(element) ? Event::EndedTransition : Event::CompletedTransition);
+    if (isConsideredHidden(element)) {
+        adjustObservedState(Event::EndedTransitionButFinalStyleIsNotDefiniteYet);
+        return;
+    }
+    adjustObservedState(isConsideredClickable(element, ElementHadRenderer::Yes) ? Event::CompletedTransitionWithClickableContent : Event::CompletedTransitionWithoutClickableContent);
 }
 
 void ContentChangeObserver::didRemoveTransition(const Element& element, CSSPropertyID propertyID)
@@ -472,7 +503,7 @@
         if (!isObservationTimeWindowActive())
             adjustStateAndNotifyContentChangeIfNeeded();
         break;
-    case Event::EndedTransition:
+    case Event::EndedTransitionButFinalStyleIsNotDefiniteYet:
         // onAnimationEnd can be called while in the middle of resolving the document (synchronously) or
         // asynchronously right before the style update is issued. It also means we don't know whether this animation ends up producing visible content yet. 
         if (m_document.inStyleRecalc()) {
@@ -481,9 +512,11 @@
         } else
             setShouldObserveNextStyleRecalc(true);
         break;
-    case Event::CompletedTransition:
+    case Event::CompletedTransitionWithClickableContent:
         // Set visibility flag on and report visible change synchronously or asynchronously depending whether we are in the middle of style recalc.
         contentVisibilityDidChange();
+        FALLTHROUGH;
+    case Event::CompletedTransitionWithoutClickableContent:
         if (m_document.inStyleRecalc())
             m_isInObservedStyleRecalc = true;
         else if (!isObservationTimeWindowActive())
@@ -520,38 +553,10 @@
         return m_wasHidden && !isConsideredHidden(m_element);
     };
 
-    if (changedFromHiddenToVisible() && isConsideredClickable())
+    if (changedFromHiddenToVisible() && isConsideredClickable(m_element, m_hadRenderer ? ElementHadRenderer::Yes : ElementHadRenderer::No))
         m_contentChangeObserver.contentVisibilityDidChange();
 }
 
-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;
-}
-
 #if ENABLE(TOUCH_EVENTS)
 ContentChangeObserver::TouchEventScope::TouchEventScope(Document& document, PlatformEvent::Type eventType)
     : m_contentChangeObserver(document.contentChangeObserver())

Modified: tags/Safari-608.1.28.1/Source/WebCore/page/ios/ContentChangeObserver.h (246242 => 246243)


--- tags/Safari-608.1.28.1/Source/WebCore/page/ios/ContentChangeObserver.h	2019-06-09 21:06:28 UTC (rev 246242)
+++ tags/Safari-608.1.28.1/Source/WebCore/page/ios/ContentChangeObserver.h	2019-06-09 21:15:50 UTC (rev 246243)
@@ -71,8 +71,6 @@
         ~StyleChangeScope();
 
     private:
-        bool isConsideredClickable() const;
-
         ContentChangeObserver& m_contentChangeObserver;
         const Element& m_element;
         bool m_wasHidden { false };
@@ -189,8 +187,9 @@
         StartedStyleRecalc,
         EndedStyleRecalc,
         AddedTransition,
-        EndedTransition,
-        CompletedTransition,
+        EndedTransitionButFinalStyleIsNotDefiniteYet,
+        CompletedTransitionWithClickableContent,
+        CompletedTransitionWithoutClickableContent,
         CanceledTransition,
         StartedFixedObservationTimeWindow,
         EndedFixedObservationTimeWindow,
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to