[ContentChangeObserver] Click event fires immediately on hover menu at Ebbets.com
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Mar 2019 04:24:40 +0000 (04:24 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Mar 2019 04:24:40 +0000 (04:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195397

Reviewed by Simon Fraser.

Source/WebCore:

This patch introduces TouchEventScope to track changes triggered by touch start.

Test: fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple.html

* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::touchEventDidStart):
(WebCore::ContentChangeObserver::touchEventDidFinish):
(WebCore::ContentChangeObserver::mouseMovedDidStart):
(WebCore::ContentChangeObserver::mouseMovedDidFinish):
(WebCore::ContentChangeObserver::adjustObservedState):
(WebCore::ContentChangeObserver::TouchEventScope::TouchEventScope):
(WebCore::ContentChangeObserver::TouchEventScope::~TouchEventScope):
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::isObservingContentChanges const):

LayoutTests:

* fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple-expected.txt: Added.
* fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/ios/ContentChangeObserver.cpp
Source/WebCore/page/ios/ContentChangeObserver.h

index fcd00eb..508c046 100644 (file)
@@ -1,5 +1,15 @@
 2019-03-07  Zalan Bujtas  <zalan@apple.com>
 
+        [ContentChangeObserver] Click event fires immediately on hover menu at Ebbets.com
+        https://bugs.webkit.org/show_bug.cgi?id=195397
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple-expected.txt: Added.
+        * fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple.html: Added.
+
+2019-03-07  Zalan Bujtas  <zalan@apple.com>
+
         [ContentChangeObserver] Introduce fixed duration content observation
         https://bugs.webkit.org/show_bug.cgi?id=195295
         <rdar://problem/48579913>
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple-expected.txt
new file mode 100644 (file)
index 0000000..66db4c9
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is not shown below.
+
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple.html b/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple.html
new file mode 100644 (file)
index 0000000..97e8aea
--- /dev/null
@@ -0,0 +1,57 @@
+<html>
+<head>
+<title>This tests the case when visible content change happens on touchstart</title>
+<script src="../../../../../resources/basic-gestures.js"></script>
+<style>
+#tapthis {
+    width: 400px;
+    height: 400px;
+    border: 1px solid green;
+}
+
+#becomesVisible {
+    visibility: hidden;
+    width: 100px;
+    height: 100px;
+    background-color: green;
+}
+</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 not shown below.</div>
+<div id=becomesVisible></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("touchstart", function( event ) {
+       becomesVisible.style.visibility = "visible";
+    if (window.testRunner)
+        setTimeout(testRunner.notifyDone(), 0);
+}, false);
+
+becomesVisible.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+}, false);
+</script>
+</body>
+</html>
index 1e493e8..00be0d4 100644 (file)
@@ -1,5 +1,27 @@
 2019-03-07  Zalan Bujtas  <zalan@apple.com>
 
+        [ContentChangeObserver] Click event fires immediately on hover menu at Ebbets.com
+        https://bugs.webkit.org/show_bug.cgi?id=195397
+
+        Reviewed by Simon Fraser.
+
+        This patch introduces TouchEventScope to track changes triggered by touch start.
+
+        Test: fast/events/touch/ios/content-observation/visibility-change-on-touch-start-simple.html
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::touchEventDidStart):
+        (WebCore::ContentChangeObserver::touchEventDidFinish):
+        (WebCore::ContentChangeObserver::mouseMovedDidStart):
+        (WebCore::ContentChangeObserver::mouseMovedDidFinish):
+        (WebCore::ContentChangeObserver::adjustObservedState):
+        (WebCore::ContentChangeObserver::TouchEventScope::TouchEventScope):
+        (WebCore::ContentChangeObserver::TouchEventScope::~TouchEventScope):
+        * page/ios/ContentChangeObserver.h:
+        (WebCore::ContentChangeObserver::isObservingContentChanges const):
+
+2019-03-07  Zalan Bujtas  <zalan@apple.com>
+
         [ContentChangeObserver] Introduce fixed duration content observation
         https://bugs.webkit.org/show_bug.cgi?id=195295
         <rdar://problem/48579913>
index 2c0706f..65d26f0 100644 (file)
@@ -154,20 +154,42 @@ void ContentChangeObserver::contentVisibilityDidChange()
     adjustObservedState(Event::ContentVisibilityChanged);
 }
 
+void ContentChangeObserver::touchEventDidStart(PlatformEvent::Type eventType)
+{
+#if ENABLE(TOUCH_EVENTS)
+    if (!m_document.settings().contentChangeObserverEnabled())
+        return;
+    if (eventType != PlatformEvent::Type::TouchStart)
+        return;
+    LOG(ContentObservation, "touchEventDidStart: touch start event started.");
+    m_touchEventIsBeingDispatched = true;
+    adjustObservedState(Event::StartedTouchStartEventDispatching);
+#endif
+}
+
+void ContentChangeObserver::touchEventDidFinish()
+{
+#if ENABLE(TOUCH_EVENTS)
+    if (!m_touchEventIsBeingDispatched)
+        return;
+    ASSERT(m_document.settings().contentChangeObserverEnabled());
+    LOG(ContentObservation, "touchEventDidFinish: touch start event finished.");
+    m_touchEventIsBeingDispatched = false;
+    adjustObservedState(Event::EndedTouchStartEventDispatching);
+#endif
+}
+
 void ContentChangeObserver::mouseMovedDidStart()
 {
 #if !ASSERT_DISABLED
     m_mouseMovedIsBeingDispatched = true;
 #endif
-    ASSERT(!m_document.hasPendingStyleRecalc());
-    clearObservedDOMTimers();
-    setShouldObserveDOMTimerScheduling(true);
     adjustObservedState(Event::StartedMouseMovedEventDispatching);
 }
 
 void ContentChangeObserver::mouseMovedDidFinish()
 {
-    setShouldObserveDOMTimerScheduling(false);
+    adjustObservedState(Event::EndedMouseMovedEventDispatching);
 #if !ASSERT_DISABLED
     m_mouseMovedIsBeingDispatched = false;
 #endif
@@ -214,8 +236,24 @@ void ContentChangeObserver::adjustObservedState(Event event)
     };
 
     switch (event) {
-    case Event::StartedMouseMovedEventDispatching:
+    case Event::StartedTouchStartEventDispatching:
         setHasNoChangeState();
+        clearObservedDOMTimers();
+        m_isMouseMovedPrecededByTouch = true;
+        setShouldObserveDOMTimerScheduling(true);
+        break;
+    case Event::StartedMouseMovedEventDispatching:
+        ASSERT(!m_document.hasPendingStyleRecalc());
+        if (!m_isMouseMovedPrecededByTouch) {
+            setHasNoChangeState();
+            clearObservedDOMTimers();
+        }
+        setShouldObserveDOMTimerScheduling(true);
+        m_isMouseMovedPrecededByTouch = false;
+        break;
+    case Event::EndedTouchStartEventDispatching:
+    case Event::EndedMouseMovedEventDispatching:
+        setShouldObserveDOMTimerScheduling(false);
         break;
     case Event::InstalledDOMTimer:
     case Event::StartedFixedObservationTimeWindow:
@@ -298,6 +336,19 @@ ContentChangeObserver::StyleChangeScope::~StyleChangeScope()
         m_contentChangeObserver.contentVisibilityDidChange();
 }
 
+#if ENABLE(TOUCH_EVENTS)
+ContentChangeObserver::TouchEventScope::TouchEventScope(Document& document, PlatformEvent::Type eventType)
+    : m_contentChangeObserver(document.contentChangeObserver())
+{
+    m_contentChangeObserver.touchEventDidStart(eventType);
+}
+
+ContentChangeObserver::TouchEventScope::~TouchEventScope()
+{
+    m_contentChangeObserver.touchEventDidFinish();
+}
+#endif
+
 ContentChangeObserver::MouseMovedScope::MouseMovedScope(Document& document)
     : m_contentChangeObserver(document.contentChangeObserver())
 {
index ec860f6..e7db158 100644 (file)
@@ -28,6 +28,7 @@
 #if PLATFORM(IOS_FAMILY)
 
 #include "Document.h"
+#include "PlatformEvent.h"
 #include "RenderStyleConstants.h"
 #include "Timer.h"
 #include "WKContentObservation.h"
@@ -65,6 +66,14 @@ public:
         Visibility m_previousImplicitVisibility;
     };
 
+    class TouchEventScope {
+    public:
+        WEBCORE_EXPORT TouchEventScope(Document&, PlatformEvent::Type);
+        WEBCORE_EXPORT ~TouchEventScope();
+    private:
+        ContentChangeObserver& m_contentChangeObserver;
+    };
+
     class MouseMovedScope {
     public:
         WEBCORE_EXPORT MouseMovedScope(Document&);
@@ -91,6 +100,9 @@ public:
     };
 
 private:
+    void touchEventDidStart(PlatformEvent::Type);
+    void touchEventDidFinish();
+
     void mouseMovedDidStart();
     void mouseMovedDidFinish();
 
@@ -110,7 +122,7 @@ private:
     void setShouldObserveNextStyleRecalc(bool);
     bool isObservingStyleRecalc() const { return m_isObservingStyleRecalc; }
 
-    bool isObservingContentChanges() const { return m_domTimerIsBeingExecuted || m_styleRecalcIsBeingExecuted || m_contentObservationTimer.isActive(); }
+    bool isObservingContentChanges() const { return m_touchEventIsBeingDispatched || m_domTimerIsBeingExecuted || m_styleRecalcIsBeingExecuted || m_contentObservationTimer.isActive(); }
 
     void cancelPendingActivities();
 
@@ -130,7 +142,10 @@ private:
     void completeDurationBasedContentObservation();
 
     enum class Event {
+        StartedTouchStartEventDispatching,
+        EndedTouchStartEventDispatching,
         StartedMouseMovedEventDispatching,
+        EndedMouseMovedEventDispatching,
         InstalledDOMTimer,
         RemovedDOMTimer,
         EndedDOMTimerExecution,
@@ -144,10 +159,12 @@ private:
     Document& m_document;
     Timer m_contentObservationTimer;
     HashSet<const DOMTimer*> m_DOMTimerList;
+    bool m_touchEventIsBeingDispatched { false };
     bool m_isObservingStyleRecalc { false };
     bool m_styleRecalcIsBeingExecuted { false };
     bool m_isObservingDOMTimerScheduling { false };
     bool m_domTimerIsBeingExecuted { false };
+    bool m_isMouseMovedPrecededByTouch { false };
 #if !ASSERT_DISABLED
     bool m_mouseMovedIsBeingDispatched { false };
 #endif