Changeset 261261 in webkit

Timestamp:

May 6, 2020, 5:18:02 PM (5 years ago)

Author:

Alan Bujtas

Message:

[ContentObservation] Shutterstock search bar is not activated on the first tap
​https://bugs.webkit.org/show_bug.cgi?id=211529
<rdar://problem/58843932>

Reviewed by Simon Fraser.

Source/WebCore:

• page/Quirks.cpp:

(WebCore::Quirks::shouldIgnoreContentObservationForSyntheticClick const):

• page/Quirks.h:

Source/WebKit:

Shutterstock has a “initial click” action which converts some “actionable” elements (<a>) to some other “actionable” elements (<button>).
If this initial click also happens to be on an element that triggers content observation (input), we see those changes as actionable content and not proceed with the click event (stay at hover).
Any subsequent click works as expected.
It’s very difficult to eliminate such false positives since all we see is that some "actionable" content is going away while some "actionable" content is being created.
This quirk ensures that the first tap on the page does not trigger content observation. (It also means that any hover menu gets submitted on the first tap, but apparently
the Shutterstock top menu bar works fine with click events.)

• WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::didStartPageTransition):

• WebProcess/WebPage/WebPage.h:
• WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::handleSyntheticClick):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/Quirks.cpp (modified) (1 diff)
• WebCore/page/Quirks.h (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebProcess/WebPage/WebPage.cpp (modified) (1 diff)
• WebKit/WebProcess/WebPage/WebPage.h (modified) (1 diff)
• WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-05-06  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentObservation] Shutterstock search bar is not activated on the first tap
+        https://bugs.webkit.org/show_bug.cgi?id=211529
+        <rdar://problem/58843932>
+
+        Reviewed by Simon Fraser.
+
+        * page/Quirks.cpp:
+        (WebCore::Quirks::shouldIgnoreContentObservationForSyntheticClick const):
+        * page/Quirks.h:
+
 2020-05-06  Jack Lee  <shihchieh_lee@apple.com>
 

trunk/Source/WebCore/page/Quirks.cpp

@@ -800 +800 @@
 }
 
-}
+bool Quirks::shouldIgnoreContentObservationForSyntheticClick(bool isFirstSyntheticClickOnPage) const
+{
+    if (!needsQuirks())
+        return false;
+
+    auto host = m_document->url().host();
+    return isFirstSyntheticClickOnPage && (equalLettersIgnoringASCIICase(host, "shutterstock.com") || host.endsWithIgnoringASCIICase(".shutterstock.com"));
+}
+
+}

trunk/Source/WebCore/page/Quirks.h

@@ -76 +76 @@
     WEBCORE_EXPORT bool shouldIgnoreAriaForFastPathContentObservationCheck() const;
     WEBCORE_EXPORT bool shouldLayOutAtMinimumWindowWidthWhenIgnoringScalingConstraints() const;
+    WEBCORE_EXPORT bool shouldIgnoreContentObservationForSyntheticClick(bool isFirstSyntheticClickOnPage) const;
 
     WEBCORE_EXPORT bool needsYouTubeMouseOutQuirk() const;

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-05-06  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentObservation] Shutterstock search bar is not activated on the first tap
+        https://bugs.webkit.org/show_bug.cgi?id=211529
+        <rdar://problem/58843932>
+
+        Reviewed by Simon Fraser.
+
+        Shutterstock has a “initial click” action which converts some “actionable” elements (<a>) to some other “actionable” elements (<button>).
+        If this initial click also happens to be on an element that triggers content observation (input), we see those changes as actionable content and not proceed with the click event (stay at hover).
+        Any subsequent click works as expected.
+        It’s very difficult to eliminate such false positives since all we see is that some "actionable" content is going away while some "actionable" content is being created.
+        This quirk ensures that the first tap on the page does not trigger content observation. (It also means that any hover menu gets submitted on the first tap, but apparently
+        the Shutterstock top menu bar works fine with click events.)
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::didStartPageTransition):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::handleSyntheticClick):
+
 2020-05-06  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -3299 +3299 @@
 #if PLATFORM(IOS_FAMILY)
     m_isShowingInputViewForFocusedElement = false;
+    // This is used to enable a first-tap quirk.
+    m_hasHandledSyntheticClick = false;
 #endif
 }

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -1968 +1968 @@
 
     bool m_isShowingInputViewForFocusedElement { false };
+    bool m_hasHandledSyntheticClick { false };
     
     enum SelectionAnchor { Start, End };

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

@@ -711 +711 @@
 void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::FloatPoint& location, OptionSet<WebEvent::Modifier> modifiers, WebCore::PointerID pointerId)
 {
-    if (!nodeRespondingToClick.document().settings().contentChangeObserverEnabled()) {
+    auto& respondingDocument = nodeRespondingToClick.document();
+    auto isFirstSyntheticClickOnPage = !m_hasHandledSyntheticClick;
+    m_hasHandledSyntheticClick = true;
+
+    if (!respondingDocument.settings().contentChangeObserverEnabled() || respondingDocument.quirks().shouldIgnoreContentObservationForSyntheticClick(isFirstSyntheticClickOnPage)) {
         completeSyntheticClick(nodeRespondingToClick, location, modifiers, WebCore::OneFingerTap, pointerId);
         return;
     }
 
-    auto& respondingDocument = nodeRespondingToClick.document();
     auto& contentChangeObserver = respondingDocument.contentChangeObserver();
     auto targetNodeWentFromHiddenToVisible = contentChangeObserver.hiddenTouchTarget() == &nodeRespondingToClick && ContentChangeObserver::isConsideredVisible(nodeRespondingToClick);