Attachment 373202 Details for Bug 199349 – Patch

[patch] Patch

bug-199349-20190630200455.patch (text/plain), 12.46 KB, created by zalan on 2019-06-30 20:04:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-06-30 20:04:57 PDT

Size: 12.46 KB

patch

obsolete

>Subversion Revision: 246791
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 704f598299e11482f5d1b3128e2bceebc42ab192..0b0fcd511d37b7e3e7ab30eabb645a0eac90d624 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-06-30  Zalan Bujtas  <zalan@apple.com>
>+
>+        Tapping on the bottom part of youtube video behaves as if controls were visible
>+        https://bugs.webkit.org/show_bug.cgi?id=199349
>+        <rdar://problem/51955744>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Synthetic click event should not be dispatched to a node that is initially hidden and becomes visible by the touchStart event.
>+
>+        Test: fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2.html
>+
>+        * dom/Node.cpp:
>+        (WebCore::Node::defaultEventHandler):
>+        * page/ios/ContentChangeObserver.cpp:
>+        (WebCore::ContentChangeObserver::isConsideredHidden):
>+        (WebCore::ContentChangeObserver::reset):
>+        (WebCore::isConsideredHidden): Deleted.
>+        * page/ios/ContentChangeObserver.h:
>+        (WebCore::ContentChangeObserver::setHiddenTouchTarget):
>+        (WebCore::ContentChangeObserver::resetHiddenTouchTarget):
>+        (WebCore::ContentChangeObserver::hiddenTouchTarget const):
>+
> 2019-06-25  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Add user agent quirk for bankofamerica.com
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4c8fee3fbb9c0ddae2517a9f5609ec77df1a8451..7df4d2e1a71ee917b000f9a9be3173d2a6b72c9f 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-06-30  Zalan Bujtas  <zalan@apple.com>
>+
>+        Tapping on the bottom part of youtube video behaves as if controls were visible
>+        https://bugs.webkit.org/show_bug.cgi?id=199349
>+        <rdar://problem/51955744>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::handleTouchEvent):
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::handleSyntheticClick):
>+        (WebKit::WebPage::completePendingSyntheticClickForContentChangeObserver):
>+        (WebKit::WebPage::completeSyntheticClick):
>+        (WebKit::WebPage::potentialTapAtPosition):
>+
> 2019-06-25  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Fully rename WebKitGTK+ -> WebKitGTK everywhere
>diff --git a/Source/WebCore/dom/Node.cpp b/Source/WebCore/dom/Node.cpp
>index fae18a95d8ba1ac2f57a2d22a47deb8c0ae3ccde..c971254d03eef1f7a533fac904ea4699df67a5e9 100644
>--- a/Source/WebCore/dom/Node.cpp
>+++ b/Source/WebCore/dom/Node.cpp
>@@ -32,6 +32,7 @@
> #include "CommonVM.h"
> #include "ComposedTreeAncestorIterator.h"
> #include "ContainerNodeAlgorithms.h"
>+#include "ContentChangeObserver.h"
> #include "ContextMenuController.h"
> #include "DOMWindow.h"
> #include "DataTransfer.h"
>@@ -2452,6 +2453,15 @@ void Node::defaultEventHandler(Event& event)
>                 frame->eventHandler().defaultWheelEventHandler(startNode, downcast<WheelEvent>(event));
> #if ENABLE(TOUCH_EVENTS) && PLATFORM(IOS_FAMILY)
>     } else if (is<TouchEvent>(event) && eventNames().isTouchRelatedEventType(document(), eventType)) {
>+        // Capture the target node's visibility state before dispatching touchStart.
>+        if (is<Element>(*this) && eventType == eventNames().touchstartEvent) {
>+            auto& contentChangeObserver = document().contentChangeObserver(); 
>+            if (ContentChangeObserver::isConsideredHidden(*this))
>+                contentChangeObserver.setHiddenTouchTarget(downcast<Element>(*this));
>+            else
>+                contentChangeObserver.resetHiddenTouchTarget();
>+        }
>+
>         RenderObject* renderer = this->renderer();
>         while (renderer && (!is<RenderBox>(*renderer) || !downcast<RenderBox>(*renderer).canBeScrolledAndHasScrollableArea()))
>             renderer = renderer->parent();
>diff --git a/Source/WebCore/page/ios/ContentChangeObserver.cpp b/Source/WebCore/page/ios/ContentChangeObserver.cpp
>index 803c4599fd688febaae409ba421ef2a53a6ea0c2..089973cc261bbb19837fa5276dab9cd059db8cdd 100644
>--- a/Source/WebCore/page/ios/ContentChangeObserver.cpp
>+++ b/Source/WebCore/page/ios/ContentChangeObserver.cpp
>@@ -43,12 +43,12 @@ namespace WebCore {
> static const Seconds maximumDelayForTimers { 400_ms };
> static const Seconds maximumDelayForTransitions { 300_ms };
> 
>-static bool isConsideredHidden(const Element& element)
>+bool ContentChangeObserver::isConsideredHidden(const Node& node)
> {
>-    if (!element.renderStyle())
>+    if (!node.renderStyle())
>         return true;
> 
>-    auto& style = *element.renderStyle();
>+    auto& style = *node.renderStyle();
>     if (style.display() == DisplayType::None)
>         return true;
> 
>@@ -80,6 +80,14 @@ static bool isConsideredHidden(const Element& element)
>     if (maxHeight.isFixed() && !maxHeight.value())
>         return true;
> 
>+    // Special case opacity, because a descendant with non-zero opacity should still be considered hidden when one of its ancetors has opacity: 0;
>+    // YouTube.com has this setup with the bottom control bar.
>+    constexpr static unsigned numberOfAncestorsToCheckForOpacity = 4;
>+    unsigned i = 0;
>+    for (auto* parent = node.parentNode(); parent && i < numberOfAncestorsToCheckForOpacity; parent = parent->parentNode(), ++i) {
>+        if (!parent->renderStyle() || !parent->renderStyle()->opacity())
>+            return true;
>+    }
>     return false;
> }
> 
>@@ -324,6 +332,7 @@ void ContentChangeObserver::reset()
> 
>     m_contentObservationTimer.stop();
>     m_elementsWithDestroyedVisibleRenderer.clear();
>+    resetHiddenTouchTarget();
> }
> 
> void ContentChangeObserver::didSuspendActiveDOMObjects()
>@@ -579,6 +588,7 @@ ContentChangeObserver::MouseMovedScope::MouseMovedScope(Document& document)
> ContentChangeObserver::MouseMovedScope::~MouseMovedScope()
> {
>     m_contentChangeObserver.mouseMovedDidFinish();
>+    m_contentChangeObserver.resetHiddenTouchTarget();
> }
> 
> ContentChangeObserver::StyleRecalcScope::StyleRecalcScope(Document& document)
>diff --git a/Source/WebCore/page/ios/ContentChangeObserver.h b/Source/WebCore/page/ios/ContentChangeObserver.h
>index 1ef64c2b343e89ae76819a0e2d6905d7918f8461..32da407e6d43e17ce2427aab6d69250e7c3026d6 100644
>--- a/Source/WebCore/page/ios/ContentChangeObserver.h
>+++ b/Source/WebCore/page/ios/ContentChangeObserver.h
>@@ -35,6 +35,7 @@
> #include "WKContentObservation.h"
> #include <wtf/HashSet.h>
> #include <wtf/Seconds.h>
>+#include <wtf/WeakPtr.h>
> 
> namespace WebCore {
> 
>@@ -48,6 +49,7 @@ public:
> 
>     WEBCORE_EXPORT void startContentObservationForDuration(Seconds duration);
>     WKContentChange observedContentChange() const { return m_observedContentState; }
>+    WEBCORE_EXPORT static bool isConsideredHidden(const Node&);
> 
>     void didInstallDOMTimer(const DOMTimer&, Seconds timeout, bool singleShot);
>     void didRemoveDOMTimer(const DOMTimer&);
>@@ -65,6 +67,10 @@ public:
> 
>     void willDestroyRenderer(const Element&);
> 
>+    void setHiddenTouchTarget(Element& targetElement) { m_hiddenTouchTargetElement = makeWeakPtr(targetElement); }
>+    void resetHiddenTouchTarget() { m_hiddenTouchTargetElement = { }; }
>+    Element* hiddenTouchTarget() const { return m_hiddenTouchTargetElement.get(); }
>+
>     class StyleChangeScope {
>     public:
>         StyleChangeScope(Document&, const Element&);
>@@ -204,6 +210,7 @@ private:
>     HashSet<const Element*> m_elementsWithTransition;
>     HashSet<const Element*> m_elementsWithDestroyedVisibleRenderer;
>     WKContentChange m_observedContentState { WKContentNoChange };
>+    WeakPtr<Element> m_hiddenTouchTargetElement;
>     bool m_touchEventIsBeingDispatched { false };
>     bool m_isWaitingForStyleRecalc { false };
>     bool m_isInObservedStyleRecalc { false };
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 044e11f017876901c81d39124bf68fde38e1cfa0..055f29297598c2117ff73d2ed7f2fa17d57474a8 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -664,18 +664,23 @@ void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::F
>     }
> 
>     auto& respondingDocument = nodeRespondingToClick.document();
>+    auto& contentChangeObserver = respondingDocument.contentChangeObserver();
>+    auto targetNodeisConsideredHidden = contentChangeObserver.hiddenTouchTarget() == &nodeRespondingToClick || ContentChangeObserver::isConsideredHidden(nodeRespondingToClick);
>     {
>         LOG_WITH_STREAM(ContentObservation, stream << "handleSyntheticClick: node(" << &nodeRespondingToClick << ") " << location);
>         ContentChangeObserver::MouseMovedScope observingScope(respondingDocument);
>         auto& mainFrame = m_page->mainFrame();
>         dispatchSyntheticMouseMove(mainFrame, location, modifiers, pointerId);
>         mainFrame.document()->updateStyleIfNeeded();
>+        if (m_isClosed)
>+            return;
>     }
> 
>-    if (m_isClosed)
>+    if (targetNodeisConsideredHidden) {
>+        LOG(ContentObservation, "handleSyntheticClick: target node is considered hidden -> hover.");
>         return;
>+    }
> 
>-    auto& contentChangeObserver = respondingDocument.contentChangeObserver();
>     auto observedContentChange = contentChangeObserver.observedContentChange();
>     auto targetNodeTriggersClick = nodeAlwaysTriggersClick(nodeRespondingToClick);
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 81a485538cb98518ebb8573c01eb8fd5a2152e79..831c5ed5112d36788275d08fcdc52b0fae632d8c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-06-30  Zalan Bujtas  <zalan@apple.com>
>+
>+        Tapping on the bottom part of youtube video behaves as if controls were visible
>+        https://bugs.webkit.org/show_bug.cgi?id=199349
>+        <rdar://problem/51955744>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2-expected.txt: Added.
>+        * fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2.html: Added.
>+
> 2019-06-25  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         Unreviewed test gardening
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..66db4c9408a6c0312dd0e2d485c01b835e7cf93d
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2-expected.txt
>@@ -0,0 +1,2 @@
>+PASS if 'clicked' text is not shown below.
>+
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2.html b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..be39d498bd412ecdb9fdb32ec60f4257813bdd52
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2.html
>@@ -0,0 +1,49 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<title>This tests the case when the touch target is initially hidden.</title>
>+<script src="../../../../../resources/basic-gestures.js"></script>
>+<style>
>+#tapthis {
>+    opacity: 0;
>+    width: 400px;
>+    height: 400px;
>+    border: 1px solid green;
>+    transition: opacity 20ms ease-out 10ms;
>+}
>+</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()">
>+<button id=tapthis></button>
>+<div>PASS if 'clicked' text is not shown below.</div>
>+<pre id=result></pre>
>+<script>
>+tapthis.addEventListener("touchstart", function( event ) {
>+    tapthis.style.opacity = "1";
>+    if (window.testRunner)
>+        setTimeout("testRunner.notifyDone()", 250);
>+}, false);
>+
>+tapthis.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked";
>+}, false);
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 199349: 373192 | 373202 | 373207 | 373208 | 373247