Title: [248911] trunk
- Revision
- 248911
- Author
- za...@apple.com
- Date
- 2019-08-20 11:58:12 -0700 (Tue, 20 Aug 2019)
Log Message
[ContentChangeObserver] isConsideredClickable should be able to process elements with no renderers
https://bugs.webkit.org/show_bug.cgi?id=200926
<rdar://problem/54519579>
Reviewed by Simon Fraser.
Source/WebCore:
This patch fixes the crash when the visible->hidden change triggers an isConsideredClickable() call with a 'display: none' element.
Test: fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash.html
* page/ios/ContentChangeObserver.cpp:
(WebCore::isConsideredClickable):
LayoutTests:
* fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash-expected.txt: Added.
* fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (248910 => 248911)
--- trunk/LayoutTests/ChangeLog 2019-08-20 18:55:34 UTC (rev 248910)
+++ trunk/LayoutTests/ChangeLog 2019-08-20 18:58:12 UTC (rev 248911)
@@ -1,3 +1,14 @@
+2019-08-20 Zalan Bujtas <za...@apple.com>
+
+ [ContentChangeObserver] isConsideredClickable should be able to process elements with no renderers
+ https://bugs.webkit.org/show_bug.cgi?id=200926
+ <rdar://problem/54519579>
+
+ Reviewed by Simon Fraser.
+
+ * fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash-expected.txt: Added.
+ * fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash.html: Added.
+
2019-08-20 Wenson Hsieh <wenson_hs...@apple.com>
Clicking the search icon on ae.com hangs the web content process
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash-expected.txt (0 => 248911)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash-expected.txt 2019-08-20 18:58:12 UTC (rev 248911)
@@ -0,0 +1,2 @@
+PASS if no crash and 'clicked' text is shown below.
+clicked
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash.html (0 => 248911)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash.html 2019-08-20 18:58:12 UTC (rev 248911)
@@ -0,0 +1,50 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<title>This tests the case when visible and non-clickable content gets removed.</title>
+<script src=""
+<style>
+#tapThis {
+ width: 400px;
+ height: 400px;
+ border: 1px solid green;
+}
+
+#alreadyVisible {
+ display: block;
+ width: 100px;
+ height: 100px;
+ background-color: green;
+}
+</style>
+<script>
+async function test() {
+ if (!window.testRunner || !testRunner.runUIScript)
+ return;
+ if (window.internals)
+ internals.settings.setContentChangeObserverEnabled(true);
+
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+
+ await UIHelper.activateElement(tapThis);
+}
+</script>
+</head>
+<body _onload_="test()">
+<div id=tapThis>PASS if no crash and 'clicked' text is shown below.</div>
+<div id=alreadyVisible></div>
+<pre id=result></pre>
+<script>
+tapThis.addEventListener("mouseover", function( event ) {
+ alreadyVisible.style.display = "none";
+}, false);
+
+tapThis.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked";
+ if (window.testRunner)
+ testRunner.notifyDone();
+}, false);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (248910 => 248911)
--- trunk/Source/WebCore/ChangeLog 2019-08-20 18:55:34 UTC (rev 248910)
+++ trunk/Source/WebCore/ChangeLog 2019-08-20 18:58:12 UTC (rev 248911)
@@ -1,3 +1,18 @@
+2019-08-20 Zalan Bujtas <za...@apple.com>
+
+ [ContentChangeObserver] isConsideredClickable should be able to process elements with no renderers
+ https://bugs.webkit.org/show_bug.cgi?id=200926
+ <rdar://problem/54519579>
+
+ Reviewed by Simon Fraser.
+
+ This patch fixes the crash when the visible->hidden change triggers an isConsideredClickable() call with a 'display: none' element.
+
+ Test: fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-crash.html
+
+ * page/ios/ContentChangeObserver.cpp:
+ (WebCore::isConsideredClickable):
+
2019-08-20 Wenson Hsieh <wenson_hs...@apple.com>
Clicking the search icon on ae.com hangs the web content process
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (248910 => 248911)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-08-20 18:55:34 UTC (rev 248910)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-08-20 18:58:12 UTC (rev 248911)
@@ -134,9 +134,9 @@
}
enum class ElementHadRenderer { No, Yes };
-static bool isConsideredClickable(const Element& newlyVisibleElement, ElementHadRenderer hadRenderer)
+static bool isConsideredClickable(const Element& candidateElement, ElementHadRenderer hadRenderer)
{
- auto& element = const_cast<Element&>(newlyVisibleElement);
+ auto& element = const_cast<Element&>(candidateElement);
if (element.isInUserAgentShadowTree())
return false;
@@ -148,10 +148,12 @@
return element.Element::willRespondToMouseClickEvents();
}
+ bool hasRenderer = element.renderer();
auto willRespondToMouseClickEvents = element.willRespondToMouseClickEvents();
- if (hadRenderer == ElementHadRenderer::No || willRespondToMouseClickEvents)
+ if (willRespondToMouseClickEvents || !hasRenderer || hadRenderer == ElementHadRenderer::No)
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.
+
+ // In case when the content already had renderers it's not sufficient to check the candidate element only since it might just be the container for the clickable content.
for (auto& descendant : descendantsOfType<RenderElement>(*element.renderer())) {
if (!descendant.element())
continue;
@@ -160,6 +162,7 @@
}
return false;
}
+
ContentChangeObserver::ContentChangeObserver(Document& document)
: m_document(document)
, m_contentObservationTimer([this] { completeDurationBasedContentObservation(); })
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
