[webkit-changes] [247015] trunk

Mon, 01 Jul 2019 12:58:35 -0700

Title: [247015] trunk
Revision
247015
Author
[email protected]
Date
2019-07-01 12:57:39 -0700 (Mon, 01 Jul 2019)

Log Message

Source/WebCore:
[iPadOS] 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 Simon Fraser.

Synthetic click event should not be dispatched to a node that is initially hidden (by opacity: 0) and becomes visible by the touchStart event.
While this behaves different from macOS where opacity: 0; content is "clickable", it impoves usability on certain sites like YouTube.com.

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):

Source/WebKit:
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 Simon Fraser.

* 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):

LayoutTests:
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 Simon Fraser.

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

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (247014 => 247015)


--- trunk/LayoutTests/ChangeLog	2019-07-01 19:32:31 UTC (rev 247014)
+++ trunk/LayoutTests/ChangeLog	2019-07-01 19:57:39 UTC (rev 247015)
@@ -1,3 +1,14 @@
+2019-07-01  Zalan Bujtas  <[email protected]>
+
+        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 Simon Fraser.
+
+        * 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-07-01  Wenson Hsieh  <[email protected]>
 
         iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea

Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2-expected.txt (0 => 247015)


--- trunk/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2-expected.txt	2019-07-01 19:57:39 UTC (rev 247015)
@@ -0,0 +1,3 @@
+
+PASS if 'clicked' text is not shown below.
+

Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2.html (0 => 247015)


--- trunk/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/opacity-change-happens-on-touchstart-with-transition2.html	2019-07-01 19:57:39 UTC (rev 247015)
@@ -0,0 +1,49 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<title>This tests the case when the touch target is initially hidden.</title>
+<script src=""
+<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>

Modified: trunk/Source/WebCore/ChangeLog (247014 => 247015)


--- trunk/Source/WebCore/ChangeLog	2019-07-01 19:32:31 UTC (rev 247014)
+++ trunk/Source/WebCore/ChangeLog	2019-07-01 19:57:39 UTC (rev 247015)
@@ -1,3 +1,27 @@
+2019-07-01  Zalan Bujtas  <[email protected]>
+
+        [iPadOS] 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 Simon Fraser.
+
+        Synthetic click event should not be dispatched to a node that is initially hidden (by opacity: 0) and becomes visible by the touchStart event.
+        While this behaves different from macOS where opacity: 0; content is "clickable", it impoves usability on certain sites like YouTube.com. 
+
+        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-28  Brent Fulgham  <[email protected]>
 
         [FTW] Build WebCore

Modified: trunk/Source/WebCore/dom/Node.cpp (247014 => 247015)


--- trunk/Source/WebCore/dom/Node.cpp	2019-07-01 19:32:31 UTC (rev 247014)
+++ trunk/Source/WebCore/dom/Node.cpp	2019-07-01 19:57:39 UTC (rev 247015)
@@ -32,6 +32,9 @@
 #include "CommonVM.h"
 #include "ComposedTreeAncestorIterator.h"
 #include "ContainerNodeAlgorithms.h"
+#if PLATFORM(IOS_FAMILY)
+#include "ContentChangeObserver.h"
+#endif
 #include "ContextMenuController.h"
 #include "DOMWindow.h"
 #include "DataTransfer.h"
@@ -2452,6 +2455,15 @@
                 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();

Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (247014 => 247015)


--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp	2019-07-01 19:32:31 UTC (rev 247014)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp	2019-07-01 19:57:39 UTC (rev 247015)
@@ -43,12 +43,12 @@
 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 @@
     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 @@
 
     m_contentObservationTimer.stop();
     m_elementsWithDestroyedVisibleRenderer.clear();
+    resetHiddenTouchTarget();
 }
 
 void ContentChangeObserver::didSuspendActiveDOMObjects()
@@ -579,6 +588,7 @@
 ContentChangeObserver::MouseMovedScope::~MouseMovedScope()
 {
     m_contentChangeObserver.mouseMovedDidFinish();
+    m_contentChangeObserver.resetHiddenTouchTarget();
 }
 
 ContentChangeObserver::StyleRecalcScope::StyleRecalcScope(Document& document)

Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.h (247014 => 247015)


--- trunk/Source/WebCore/page/ios/ContentChangeObserver.h	2019-07-01 19:32:31 UTC (rev 247014)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.h	2019-07-01 19:57:39 UTC (rev 247015)
@@ -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 @@
 
     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 @@
 
     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 @@
     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 };

Modified: trunk/Source/WebKit/ChangeLog (247014 => 247015)


--- trunk/Source/WebKit/ChangeLog	2019-07-01 19:32:31 UTC (rev 247014)
+++ trunk/Source/WebKit/ChangeLog	2019-07-01 19:57:39 UTC (rev 247015)
@@ -1,3 +1,20 @@
+2019-07-01  Zalan Bujtas  <[email protected]>
+
+        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 Simon Fraser.
+
+        * 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-07-01  Wenson Hsieh  <[email protected]>
 
         iOS: REGRESSION(async scroll): Caret doesn't scroll when scrolling textarea

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (247014 => 247015)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2019-07-01 19:32:31 UTC (rev 247014)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2019-07-01 19:57:39 UTC (rev 247015)
@@ -664,6 +664,8 @@
     }
 
     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);
@@ -670,12 +672,15 @@
         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);
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to