Changeset 247200 in webkit

Timestamp:

Jul 7, 2019 7:52:17 AM (5 years ago)

Author:

Alan Bujtas

Message:

[ContentChangeObserver] Difficult to control videos on iqiyi.com as the actions are mouse hover
​https://bugs.webkit.org/show_bug.cgi?id=199542
<rdar://problem/51886813>

Reviewed by Simon Fraser.

Source/WebCore:

Decouple isVisuallyHidden and isConsideredVisible. Just because an element is not visually hidden (1px wide content)
it is not necessarily qualified to be visible in the context of hover heuristic (e.g. iqiyi.com brings up a 1px wide
clickable element when hovering over the scrubber. This element is clearly not designed to be actionable.)

Tests: fast/events/touch/ios/content-observation/tap-on-1px-height-content.html

fast/events/touch/ios/content-observation/tap-on-1px-width-content.html

• dom/Node.cpp:

(WebCore::Node::defaultEventHandler):

• page/ios/ContentChangeObserver.cpp:

(WebCore::ContentChangeObserver::isVisuallyHidden):
(WebCore::ContentChangeObserver::isConsideredVisible):
(WebCore::ContentChangeObserver::didAddTransition):
(WebCore::ContentChangeObserver::didFinishTransition):
(WebCore::ContentChangeObserver::willDestroyRenderer):
(WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
(WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
(WebCore::ContentChangeObserver::isConsideredHidden): Deleted.

• page/ios/ContentChangeObserver.h:

Source/WebKit:

• WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::handleSyntheticClick):

LayoutTests:

• fast/events/touch/ios/content-observation/tap-on-1px-height-content-expected.txt: Added.
• fast/events/touch/ios/content-observation/tap-on-1px-height-content.html: Added.
• fast/events/touch/ios/content-observation/tap-on-1px-width-content-expected.txt: Added.
• fast/events/touch/ios/content-observation/tap-on-1px-width-content.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/touch/ios/content-observation/tap-on-1px-height-content-expected.txt (added)
• LayoutTests/fast/events/touch/ios/content-observation/tap-on-1px-height-content.html (added)
• LayoutTests/fast/events/touch/ios/content-observation/tap-on-1px-width-content-expected.txt (added)
• LayoutTests/fast/events/touch/ios/content-observation/tap-on-1px-width-content.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Node.cpp (modified) (1 diff)
• Source/WebCore/page/ios/ContentChangeObserver.cpp (modified) (7 diffs)
• Source/WebCore/page/ios/ContentChangeObserver.h (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-07-07  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Difficult to control videos on iqiyi.com as the actions are mouse hover
+        https://bugs.webkit.org/show_bug.cgi?id=199542
+        <rdar://problem/51886813>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/content-observation/tap-on-1px-height-content-expected.txt: Added.
+        * fast/events/touch/ios/content-observation/tap-on-1px-height-content.html: Added.
+        * fast/events/touch/ios/content-observation/tap-on-1px-width-content-expected.txt: Added.
+        * fast/events/touch/ios/content-observation/tap-on-1px-width-content.html: Added.
+
 2019-07-06  Cathie Chen  <cathiechen@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-07-07  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Difficult to control videos on iqiyi.com as the actions are mouse hover
+        https://bugs.webkit.org/show_bug.cgi?id=199542
+        <rdar://problem/51886813>
+
+        Reviewed by Simon Fraser.
+
+        Decouple isVisuallyHidden and isConsideredVisible. Just because an element is not visually hidden (1px wide content)
+        it is not necessarily qualified to be visible in the context of hover heuristic (e.g. iqiyi.com brings up a 1px wide
+        clickable element when hovering over the scrubber. This element is clearly not designed to be actionable.)
+
+        Tests: fast/events/touch/ios/content-observation/tap-on-1px-height-content.html
+               fast/events/touch/ios/content-observation/tap-on-1px-width-content.html
+
+        * dom/Node.cpp:
+        (WebCore::Node::defaultEventHandler):
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::isVisuallyHidden):
+        (WebCore::ContentChangeObserver::isConsideredVisible):
+        (WebCore::ContentChangeObserver::didAddTransition):
+        (WebCore::ContentChangeObserver::didFinishTransition):
+        (WebCore::ContentChangeObserver::willDestroyRenderer):
+        (WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
+        (WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
+        (WebCore::ContentChangeObserver::isConsideredHidden): Deleted.
+        * page/ios/ContentChangeObserver.h:
+
 2019-07-07  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/dom/Node.cpp

@@ -2459 +2459 @@
         if (is<Element>(*this) && eventType == eventNames().touchstartEvent) {
             auto& contentChangeObserver = document().contentChangeObserver(); 
-            if (ContentChangeObserver::isConsideredHidden(*this))
+            if (ContentChangeObserver::isVisuallyHidden(*this))
                 contentChangeObserver.setHiddenTouchTarget(downcast<Element>(*this));
             else

trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp

@@ -44 +44 @@
 static const Seconds maximumDelayForTransitions { 300_ms };
 
-bool ContentChangeObserver::isConsideredHidden(const Node& node)
+bool ContentChangeObserver::isVisuallyHidden(const Node& node)
 {
     if (!node.renderStyle())
@@ -90 +90 @@
     }
     return false;
+}
+
+bool ContentChangeObserver::isConsideredVisible(const Node& node)
+{
+    if (isVisuallyHidden(node))
+        return false;
+
+    auto& style = *node.renderStyle();
+    auto width = style.logicalWidth();
+    // 1px width or height content is not considered visible.
+    if (width.isFixed() && width.value() <= 1)
+        return false;
+
+    auto height = style.logicalHeight();
+    if (height.isFixed() && height.value() <= 1)
+        return false;
+
+    return true;
 }
 
@@ -176 +194 @@
     if (transitionEnd > maximumDelayForTransitions)
         return;
-    if (!isConsideredHidden(element))
+    if (!isVisuallyHidden(element))
         return;
     // In case of multiple transitions, the first tranistion wins (and it has to produce a visible content change in order to show up as hover).
@@ -199 +217 @@
         if (!weakThis || !targetElement)
             return;
-        if (isConsideredHidden(*targetElement)) {
+        if (isVisuallyHidden(*targetElement)) {
             weakThis->adjustObservedState(Event::EndedTransitionButFinalStyleIsNotDefiniteYet);
             return;
@@ -363 +381 @@
     LOG_WITH_STREAM(ContentObservation, stream << "willDestroyRenderer element: " << &element);
 
-    if (!isConsideredHidden(element))
+    if (!isVisuallyHidden(element))
         m_elementsWithDestroyedVisibleRenderer.add(&element);
 }
@@ -559 +577 @@
 {
     if (m_contentChangeObserver.shouldObserveVisibilityChangeForElement(element))
-        m_wasHidden = isConsideredHidden(m_element);
+        m_wasHidden = isVisuallyHidden(m_element);
 }
 
@@ -565 +583 @@
 {
     auto changedFromHiddenToVisible = [&] {
-        return m_wasHidden && !isConsideredHidden(m_element);
+        return m_wasHidden && isConsideredVisible(m_element);
     };
 

trunk/Source/WebCore/page/ios/ContentChangeObserver.h

@@ -50 +50 @@
     WEBCORE_EXPORT void startContentObservationForDuration(Seconds duration);
     WKContentChange observedContentChange() const { return m_observedContentState; }
-    WEBCORE_EXPORT static bool isConsideredHidden(const Node&);
+    WEBCORE_EXPORT static bool isConsideredVisible(const Node&);
+    static bool isVisuallyHidden(const Node&);
 
     void didInstallDOMTimer(const DOMTimer&, Seconds timeout, bool singleShot);

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-07-07  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Difficult to control videos on iqiyi.com as the actions are mouse hover
+        https://bugs.webkit.org/show_bug.cgi?id=199542
+        <rdar://problem/51886813>
+
+        Reviewed by Simon Fraser.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::handleSyntheticClick):
+
 2019-07-06  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -670 +670 @@
     auto& respondingDocument = nodeRespondingToClick.document();
     auto& contentChangeObserver = respondingDocument.contentChangeObserver();
-    auto targetNodeWentFromHiddenToVisible = contentChangeObserver.hiddenTouchTarget() == &nodeRespondingToClick && !ContentChangeObserver::isConsideredHidden(nodeRespondingToClick);
+    auto targetNodeWentFromHiddenToVisible = contentChangeObserver.hiddenTouchTarget() == &nodeRespondingToClick && ContentChangeObserver::isConsideredVisible(nodeRespondingToClick);
     {
         LOG_WITH_STREAM(ContentObservation, stream << "handleSyntheticClick: node(" << &nodeRespondingToClick << ") " << location);