Diff
Modified: branches/safari-608.1.27-branch/LayoutTests/ChangeLog (246190 => 246191)
--- branches/safari-608.1.27-branch/LayoutTests/ChangeLog 2019-06-07 06:09:40 UTC (rev 246190)
+++ branches/safari-608.1.27-branch/LayoutTests/ChangeLog 2019-06-07 06:26:25 UTC (rev 246191)
@@ -1,3 +1,45 @@
+2019-06-06 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-02 Babak Shafiei <[email protected]>
Cherry-pick r245989. rdar://problem/51283532
Added: branches/safari-608.1.27-branch/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt (0 => 246191)
--- branches/safari-608.1.27-branch/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt (rev 0)
+++ branches/safari-608.1.27-branch/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable-expected.txt 2019-06-07 06:26:25 UTC (rev 246191)
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is shown below.
+clicked
Added: branches/safari-608.1.27-branch/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html (0 => 246191)
--- branches/safari-608.1.27-branch/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html (rev 0)
+++ branches/safari-608.1.27-branch/LayoutTests/fast/events/touch/ios/content-observation/100ms-delay-10ms-transition-on-mousemove-no-clickable.html 2019-06-07 06:26:25 UTC (rev 246191)
@@ -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: branches/safari-608.1.27-branch/Source/WebCore/ChangeLog (246190 => 246191)
--- branches/safari-608.1.27-branch/Source/WebCore/ChangeLog 2019-06-07 06:09:40 UTC (rev 246190)
+++ branches/safari-608.1.27-branch/Source/WebCore/ChangeLog 2019-06-07 06:26:25 UTC (rev 246191)
@@ -1,3 +1,56 @@
+2019-06-06 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 Babak Shafiei <[email protected]>
Cherry-pick r246099. rdar://problem/48006291
Modified: branches/safari-608.1.27-branch/Source/WebCore/page/ios/ContentChangeObserver.cpp (246190 => 246191)
--- branches/safari-608.1.27-branch/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-06-07 06:09:40 UTC (rev 246190)
+++ branches/safari-608.1.27-branch/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-06-07 06:26:25 UTC (rev 246191)
@@ -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: branches/safari-608.1.27-branch/Source/WebCore/page/ios/ContentChangeObserver.h (246190 => 246191)
--- branches/safari-608.1.27-branch/Source/WebCore/page/ios/ContentChangeObserver.h 2019-06-07 06:09:40 UTC (rev 246190)
+++ branches/safari-608.1.27-branch/Source/WebCore/page/ios/ContentChangeObserver.h 2019-06-07 06:26:25 UTC (rev 246191)
@@ -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,
