Modified: trunk/LayoutTests/ChangeLog (248749 => 248750)
--- trunk/LayoutTests/ChangeLog 2019-08-15 22:15:29 UTC (rev 248749)
+++ trunk/LayoutTests/ChangeLog 2019-08-15 22:18:56 UTC (rev 248750)
@@ -1,3 +1,14 @@
+2019-08-15 Zalan Bujtas <za...@apple.com>
+
+ [ContentChangeObserver] Dispatch synthetic click when the visibility candidate element becomes hidden again.
+ https://bugs.webkit.org/show_bug.cgi?id=200773
+ <rdar://problem/54351728>
+
+ Reviewed by Simon Fraser.
+
+ * fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-expected.html: Added.
+ * fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html: Added.
+
2019-08-15 Robin Morisset <rmoris...@apple.com>
[WHLSL] Don't accept operator&& or operator|| in the Lexer
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-expected.txt (0 => 248750)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-expected.txt 2019-08-15 22:18:56 UTC (rev 248750)
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is shown below.
+clicked
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html (0 => 248750)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html 2019-08-15 22:18:56 UTC (rev 248750)
@@ -0,0 +1,58 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<title>This tests the case when visible and actionable content shows up and gets destroyed right away.</title>
+<script src=""
+<style>
+#tapThis {
+ width: 400px;
+ height: 400px;
+ border: 1px solid green;
+}
+
+#willBecomeVisibleMomentarily {
+ display: none;
+ 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();
+
+ await UIHelper.activateElement(tapThis);
+}
+</script>
+</head>
+<body _onload_="test()">
+<div id=tapThis>PASS if 'clicked' text is shown below.</div>
+<div id=willBecomeVisibleMomentarily></div>
+<pre id=result></pre>
+<script>
+tapThis.addEventListener("touchstart", function( event ) {
+ willBecomeVisibleMomentarily.style.display = "block";
+}, false);
+
+tapThis.addEventListener("mouseover", function( event ) {
+ willBecomeVisibleMomentarily.style.display = "none";
+}, false);
+
+willBecomeVisibleMomentarily.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked willBecomeVisibleMomentarily";
+}, false);
+
+tapThis.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked";
+ if (window.testRunner)
+ testRunner.notifyDone();
+}, false);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (248749 => 248750)
--- trunk/Source/WebCore/ChangeLog 2019-08-15 22:15:29 UTC (rev 248749)
+++ trunk/Source/WebCore/ChangeLog 2019-08-15 22:18:56 UTC (rev 248750)
@@ -1,3 +1,39 @@
+2019-08-15 Zalan Bujtas <za...@apple.com>
+
+ [ContentChangeObserver] Dispatch synthetic click when the visibility candidate element becomes hidden again.
+ https://bugs.webkit.org/show_bug.cgi?id=200773
+ <rdar://problem/54351728>
+
+ Reviewed by Simon Fraser.
+
+ This patch fixes the case when the candidate element (going from hidden to visible) becomes hidden by the end of the observation window. It essentially means that no visible change has happened
+ and we should proceed with dispatching the synthetic click event.
+ We now keep track of the candidate element and reset the visiblity state when it loses its renderer.
+
+ Test: fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html
+
+ * page/ios/ContentChangeObserver.cpp:
+ (WebCore::ContentChangeObserver::didAddTransition):
+ (WebCore::ContentChangeObserver::didFinishTransition):
+ (WebCore::ContentChangeObserver::didInstallDOMTimer):
+ (WebCore::ContentChangeObserver::reset):
+ (WebCore::ContentChangeObserver::rendererWillBeDestroyed):
+ (WebCore::ContentChangeObserver::contentVisibilityDidChange):
+ (WebCore::ContentChangeObserver::touchEventDidStart):
+ (WebCore::ContentChangeObserver::touchEventDidFinish):
+ (WebCore::ContentChangeObserver::mouseMovedDidStart):
+ (WebCore::ContentChangeObserver::mouseMovedDidFinish):
+ (WebCore::ContentChangeObserver::adjustObservedState):
+ (WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
+ (WebCore::ContentChangeObserver::hasDeterminateState const): Deleted.
+ * page/ios/ContentChangeObserver.h:
+ (WebCore::ContentChangeObserver::hasObservedTransition const):
+ (WebCore::ContentChangeObserver::setTouchEventIsBeingDispatched):
+ (WebCore::ContentChangeObserver::isTouchEventBeingDispatched const):
+ (WebCore::ContentChangeObserver::setMouseMovedEventIsBeingDispatched):
+ (WebCore::ContentChangeObserver::isMouseMovedEventBeingDispatched const):
+ (WebCore::ContentChangeObserver::isObservingContentChanges const):
+
2019-08-15 Justin Fan <justin_...@apple.com>
Unreviewed suggested patch follow-up to https://bugs.webkit.org/show_bug.cgi?id=200740.
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (248749 => 248750)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-08-15 22:15:29 UTC (rev 248749)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-08-15 22:18:56 UTC (rev 248750)
@@ -214,6 +214,8 @@
return;
if (hasVisibleChangeState())
return;
+ if (!isObservingContentChanges())
+ return;
if (!isObservingTransitions())
return;
if (!transition.isDurationSet() || !transition.isPropertySet())
@@ -251,7 +253,7 @@
return;
}
if (isConsideredClickable(*targetElement, ElementHadRenderer::Yes))
- weakThis->contentVisibilityDidChange();
+ weakThis->contentVisibilityDidChange(*targetElement);
weakThis->adjustObservedState(Event::CompletedTransition);
});
}
@@ -271,14 +273,16 @@
{
if (!m_document.settings().contentChangeObserverEnabled())
return;
- if (m_document.activeDOMObjectsAreSuspended())
+ if (!isObservingContentChanges())
return;
- if (timeout > maximumDelayForTimers || !singleShot)
- return;
if (!isObservingDOMTimerScheduling())
return;
if (hasVisibleChangeState())
return;
+ if (m_document.activeDOMObjectsAreSuspended())
+ return;
+ if (timeout > maximumDelayForTimers || !singleShot)
+ return;
LOG_WITH_STREAM(ContentObservation, stream << "didInstallDOMTimer: register this timer: (" << &timer << ") and observe when it fires.");
registerDOMTimer(timer);
@@ -359,13 +363,16 @@
{
stopObservingPendingActivities();
setHasNoChangeState();
+
+ setTouchEventIsBeingDispatched(false);
setIsBetweenTouchEndAndMouseMoved(false);
+ setMouseMovedEventIsBeingDispatched(false);
- m_touchEventIsBeingDispatched = false;
m_isInObservedStyleRecalc = false;
m_observedDomTimerIsBeingExecuted = false;
- m_mouseMovedEventIsBeingDispatched = false;
+ m_visibilityCandidateElement = { };
+
m_contentObservationTimer.stop();
m_elementsWithDestroyedVisibleRenderer.clear();
resetHiddenTouchTarget();
@@ -389,17 +396,24 @@
return;
if (!isObservingContentChanges())
return;
- if (hasVisibleChangeState())
- return;
LOG_WITH_STREAM(ContentObservation, stream << "rendererWillBeDestroyed element: " << &element);
if (!isVisuallyHidden(element))
m_elementsWithDestroyedVisibleRenderer.add(&element);
+ // Candidate element is no longer visible.
+ if (m_visibilityCandidateElement == &element) {
+ // FIXME: We should also check for other type of visiblity changes.
+ ASSERT(hasVisibleChangeState());
+ m_visibilityCandidateElement = { };
+ setHasIndeterminateState();
+ }
}
-void ContentChangeObserver::contentVisibilityDidChange()
+void ContentChangeObserver::contentVisibilityDidChange(const Element& element)
{
LOG(ContentObservation, "contentVisibilityDidChange: visible content change did happen.");
+ // FIXME: This should evolve into a list of candidate elements.
+ m_visibilityCandidateElement = makeWeakPtr(element);
adjustObservedState(Event::ContentVisibilityChanged);
}
@@ -411,7 +425,7 @@
if (eventType != PlatformEvent::Type::TouchStart)
return;
LOG(ContentObservation, "touchEventDidStart: touch start event started.");
- m_touchEventIsBeingDispatched = true;
+ setTouchEventIsBeingDispatched(true);
adjustObservedState(Event::StartedTouchStartEventDispatching);
#else
UNUSED_PARAM(eventType);
@@ -421,11 +435,11 @@
void ContentChangeObserver::touchEventDidFinish()
{
#if ENABLE(TOUCH_EVENTS)
- if (!m_touchEventIsBeingDispatched)
+ if (!isTouchEventBeingDispatched())
return;
ASSERT(m_document.settings().contentChangeObserverEnabled());
LOG(ContentObservation, "touchEventDidFinish: touch start event finished.");
- m_touchEventIsBeingDispatched = false;
+ setTouchEventIsBeingDispatched(false);
adjustObservedState(Event::EndedTouchStartEventDispatching);
#endif
}
@@ -435,18 +449,18 @@
if (!m_document.settings().contentChangeObserverEnabled())
return;
LOG(ContentObservation, "mouseMovedDidStart: mouseMoved started.");
- m_mouseMovedEventIsBeingDispatched = true;
+ setMouseMovedEventIsBeingDispatched(true);
adjustObservedState(Event::StartedMouseMovedEventDispatching);
}
void ContentChangeObserver::mouseMovedDidFinish()
{
- if (!m_mouseMovedEventIsBeingDispatched)
+ if (!isMouseMovedEventBeingDispatched())
return;
ASSERT(m_document.settings().contentChangeObserverEnabled());
LOG(ContentObservation, "mouseMovedDidFinish: mouseMoved finished.");
adjustObservedState(Event::EndedMouseMovedEventDispatching);
- m_mouseMovedEventIsBeingDispatched = false;
+ setMouseMovedEventIsBeingDispatched(false);
}
void ContentChangeObserver::setShouldObserveNextStyleRecalc(bool shouldObserve)
@@ -456,13 +470,6 @@
m_isWaitingForStyleRecalc = shouldObserve;
}
-bool ContentChangeObserver::hasDeterminateState() const
-{
- if (hasVisibleChangeState())
- return true;
- return observedContentChange() == WKContentNoChange && !hasPendingActivity();
-}
-
void ContentChangeObserver::adjustObservedState(Event event)
{
auto resetToStartObserving = [&] {
@@ -477,27 +484,34 @@
};
auto notifyClientIfNeeded = [&] {
- // First demote to "no change" if there's no pending activity anymore.
- if (observedContentChange() == WKContentIndeterminateChange && !hasPendingActivity())
- setHasNoChangeState();
-
+ if (isTouchEventBeingDispatched()) {
+ LOG(ContentObservation, "notifyClientIfNeeded: Touch event is being dispatched. No need to notify the client.");
+ return;
+ }
if (isBetweenTouchEndAndMouseMoved()) {
- LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: Not reached mouseMoved yet. No need to notify the client.");
+ LOG(ContentObservation, "notifyClientIfNeeded: Not reached mouseMoved yet. No need to notify the client.");
return;
}
- if (m_mouseMovedEventIsBeingDispatched) {
- LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: in mouseMoved call. No need to notify the client.");
+ if (isMouseMovedEventBeingDispatched()) {
+ LOG(ContentObservation, "notifyClientIfNeeded: in mouseMoved call. No need to notify the client.");
return;
}
if (isObservationTimeWindowActive()) {
- LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: Inside the fixed window observation. No need to notify the client.");
+ LOG(ContentObservation, "notifyClientIfNeeded: Inside the fixed window observation. No need to notify the client.");
return;
}
- if (!hasDeterminateState()) {
- LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: not in a determined state yet.");
+
+ // The fixed observation window (which is the final step in content observation) is closed and now we check if are still waiting for timers or animations to finish.
+ if (hasPendingActivity()) {
+ LOG(ContentObservation, "notifyClientIfNeeded: We are still waiting on some events.");
return;
}
- LOG_WITH_STREAM(ContentObservation, stream << "adjustStateAndNotifyContentChangeIfNeeded: sending observedContentChange ->" << observedContentChange());
+
+ // First demote to "no change" because we've got no pending activity anymore.
+ if (observedContentChange() == WKContentIndeterminateChange)
+ setHasNoChangeState();
+
+ LOG_WITH_STREAM(ContentObservation, stream << "notifyClientIfNeeded: sending observedContentChange ->" << observedContentChange());
ASSERT(m_document.page());
ASSERT(m_document.frame());
m_document.page()->chrome().client().didFinishContentChangeObserving(*m_document.frame(), observedContentChange());
@@ -595,7 +609,7 @@
}
// Either the page decided to call preventDefault on the touch action or the tap gesture evolved to some other gesture (long press, double tap).
if (event == Event::WillNotProceedWithClick) {
- reset();
+ stopContentObservation();
return;
}
// The page produced an visible change on an actionable content.
@@ -628,7 +642,7 @@
};
if (changedFromHiddenToVisible() && isConsideredClickable(m_element, m_hadRenderer ? ElementHadRenderer::Yes : ElementHadRenderer::No))
- m_contentChangeObserver.contentVisibilityDidChange();
+ m_contentChangeObserver.contentVisibilityDidChange(m_element);
}
#if ENABLE(TOUCH_EVENTS)
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.h (248749 => 248750)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2019-08-15 22:15:29 UTC (rev 248749)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2019-08-15 22:18:56 UTC (rev 248750)
@@ -128,7 +128,7 @@
void didRecognizeLongPress();
- void contentVisibilityDidChange();
+ void contentVisibilityDidChange(const Element&);
void setShouldObserveDOMTimerSchedulingAndTransitions(bool);
bool isObservingDOMTimerScheduling() const { return m_isObservingDOMTimerScheduling; }
@@ -159,11 +159,16 @@
bool hasVisibleChangeState() const { return observedContentChange() == WKContentVisibilityChange; }
bool hasObservedDOMTimer() const { return !m_DOMTimerList.isEmpty(); }
bool hasObservedTransition() const { return !m_elementsWithTransition.isEmpty(); }
- bool hasDeterminateState() const;
void setIsBetweenTouchEndAndMouseMoved(bool isBetween) { m_isBetweenTouchEndAndMouseMoved = isBetween; }
bool isBetweenTouchEndAndMouseMoved() const { return m_isBetweenTouchEndAndMouseMoved; }
+ void setTouchEventIsBeingDispatched(bool dispatching) { m_touchEventIsBeingDispatched = dispatching; }
+ bool isTouchEventBeingDispatched() const { return m_touchEventIsBeingDispatched; }
+
+ void setMouseMovedEventIsBeingDispatched(bool dispatching) { m_mouseMovedEventIsBeingDispatched = dispatching; }
+ bool isMouseMovedEventBeingDispatched() const { return m_mouseMovedEventIsBeingDispatched; }
+
bool hasPendingActivity() const { return hasObservedDOMTimer() || hasObservedTransition() || m_isWaitingForStyleRecalc || isObservationTimeWindowActive(); }
bool isObservationTimeWindowActive() const { return m_contentObservationTimer.isActive(); }
@@ -202,6 +207,7 @@
HashSet<const Element*> m_elementsWithDestroyedVisibleRenderer;
WKContentChange m_observedContentState { WKContentNoChange };
WeakPtr<Element> m_hiddenTouchTargetElement;
+ WeakPtr<Element> m_visibilityCandidateElement;
bool m_touchEventIsBeingDispatched { false };
bool m_isWaitingForStyleRecalc { false };
bool m_isInObservedStyleRecalc { false };
@@ -214,9 +220,9 @@
inline bool ContentChangeObserver::isObservingContentChanges() const
{
- return m_touchEventIsBeingDispatched
- || m_isBetweenTouchEndAndMouseMoved
- || m_mouseMovedEventIsBeingDispatched
+ return isTouchEventBeingDispatched()
+ || isBetweenTouchEndAndMouseMoved()
+ || isMouseMovedEventBeingDispatched()
|| m_observedDomTimerIsBeingExecuted
|| m_isInObservedStyleRecalc
|| isObservationTimeWindowActive();
