[ContentChangeObserver] HTMLImageElement::willRespondToMouseClickEvents returns quirk...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2019 22:18:41 +0000 (22:18 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2019 22:18:41 +0000 (22:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195657
<rdar://problem/48834987>

Reviewed by Simon Fraser.

Source/WebCore:

Images should not trigger hover by default (only when they actually respond to mouse events).

Test: fast/events/touch/ios/content-observation/visibility-change-with-image-content.html

* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::StyleChangeScope::isConsideredClickable const):

LayoutTests:

* fast/events/touch/ios/content-observation/visibility-change-with-image-content-expected.txt: Added.
* fast/events/touch/ios/content-observation/visibility-change-with-image-content.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243017 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/content-observation/visibility-change-with-image-content-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/content-observation/visibility-change-with-image-content.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/page/ios/ContentChangeObserver.cpp

index 9b8d86f..7fb72af 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-15  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] HTMLImageElement::willRespondToMouseClickEvents returns quirk value.
+        https://bugs.webkit.org/show_bug.cgi?id=195657
+        <rdar://problem/48834987>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/content-observation/visibility-change-with-image-content-expected.txt: Added.
+        * fast/events/touch/ios/content-observation/visibility-change-with-image-content.html: Added.
+
 2019-03-15  Dean Jackson  <dino@apple.com>
 
         Provide an option for an always-on fast click mode in iOS
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-with-image-content-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-with-image-content-expected.txt
new file mode 100644 (file)
index 0000000..31954a3
--- /dev/null
@@ -0,0 +1,3 @@
+PASS if 'clicked' text is shown below.
+
+clicked
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-with-image-content.html b/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-with-image-content.html
new file mode 100644 (file)
index 0000000..b951bbd
--- /dev/null
@@ -0,0 +1,53 @@
+<html>
+<head>
+<title>This tests the case when visible content change includes image content</title>
+<script src="../../../../../resources/basic-gestures.js"></script>
+<style>
+#tapthis {
+    width: 400px;
+    height: 400px;
+    border: 1px solid green;
+}
+
+#becomesVisible {
+    display: none;
+    width: 100px;
+    height: 100px;
+    background-color: blue;
+}
+</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>
+<img src="../../../resources/greenbox30.png" id=becomesVisible>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mousemove", function( event ) {
+    becomesVisible.style.display = "inline";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+    document.body.offsetHeight;
+    testRunner.notifyDone();
+}, false);
+</script>
+</body>
+</html>
index 02160d4..e9a89f8 100644 (file)
@@ -1,3 +1,18 @@
+2019-03-15  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] HTMLImageElement::willRespondToMouseClickEvents returns quirk value.
+        https://bugs.webkit.org/show_bug.cgi?id=195657
+        <rdar://problem/48834987>
+
+        Reviewed by Simon Fraser.
+
+        Images should not trigger hover by default (only when they actually respond to mouse events).
+
+        Test: fast/events/touch/ios/content-observation/visibility-change-with-image-content.html
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::StyleChangeScope::isConsideredClickable const):
+
 2019-03-15  Jer Noble  <jer.noble@apple.com>
 
         Add a "supportedConfiguration" dictionary to MediaCapabilitiesDecodingInfo and MediaCapabilitiesEncodingInfo
index 81a92e5..d047978 100644 (file)
@@ -790,7 +790,7 @@ bool HTMLImageElement::childShouldCreateRenderer(const Node& child) const
 #endif // ENABLE(SERVICE_CONTROLS)
 
 #if PLATFORM(IOS_FAMILY)
-// FIXME: This is a workaround for <rdar://problem/7725158>. We should find a better place for the touchCalloutEnabled() logic.
+// FIXME: We should find a better place for the touch callout logic. See rdar://problem/48937767.
 bool HTMLImageElement::willRespondToMouseClickEvents()
 {
     auto renderer = this->renderer();
index c48d8ac..8bde2d3 100644 (file)
@@ -30,6 +30,7 @@
 #include "ChromeClient.h"
 #include "DOMTimer.h"
 #include "Document.h"
+#include "HTMLImageElement.h"
 #include "Logging.h"
 #include "NodeRenderStyle.h"
 #include "Page.h"
@@ -393,13 +394,19 @@ bool ContentChangeObserver::StyleChangeScope::isConsideredClickable() const
 {
     if (m_element.isInUserAgentShadowTree())
         return false;
-    if (!m_hadRenderer)
-        return const_cast<Element&>(m_element).willRespondToMouseClickEvents();
-    ASSERT(m_element.renderer());
-    if (const_cast<Element&>(m_element).willRespondToMouseClickEvents())
-        return true;
+
+    auto& element = const_cast<Element&>(m_element);
+    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.  
-    for (auto& descendant : descendantsOfType<RenderElement>(*m_element.renderer())) {
+    ASSERT(m_element.renderer());
+    for (auto& descendant : descendantsOfType<RenderElement>(*element.renderer())) {
         if (descendant.element()->willRespondToMouseClickEvents())
             return true;
     }