Title: [242661] trunk
- Revision
- 242661
- Author
- [email protected]
- Date
- 2019-03-08 15:51:05 -0800 (Fri, 08 Mar 2019)
Log Message
[ContentChangeObserver] Expand "isConsideredClickable" to descendants
https://bugs.webkit.org/show_bug.cgi?id=195478
<rdar://problem/48724935>
Reviewed by Simon Fraser.
Source/WebCore:
In StyleChangeScope we try to figure out whether newly visible content should stick (menu panes etc) by checking if it is clickable.
This works fine as long as all the visible elements are gaining new renderers through this style update processs.
However when an element becomes visible by a change other than display: (not)none, it's not sufficient to just check the element itself,
since it might not respond to click at all, while its descendants do.
A concrete example is a max-height value change on usps.com, where the max-height is on a container (menu pane).
This container itself is not clickable while most of its children are (menu items).
Test: fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container.html
* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
(WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
(WebCore::ContentChangeObserver::StyleChangeScope::isConsideredHidden const):
(WebCore::ContentChangeObserver::StyleChangeScope::isConsideredClickable const):
(WebCore::isConsideredHidden): Deleted.
* page/ios/ContentChangeObserver.h:
LayoutTests:
* fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container-expected.txt: Added.
* fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (242660 => 242661)
--- trunk/LayoutTests/ChangeLog 2019-03-08 23:31:38 UTC (rev 242660)
+++ trunk/LayoutTests/ChangeLog 2019-03-08 23:51:05 UTC (rev 242661)
@@ -1,3 +1,14 @@
+2019-03-08 Zalan Bujtas <[email protected]>
+
+ [ContentChangeObserver] Expand "isConsideredClickable" to descendants
+ https://bugs.webkit.org/show_bug.cgi?id=195478
+ <rdar://problem/48724935>
+
+ Reviewed by Simon Fraser.
+
+ * fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container-expected.txt: Added.
+ * fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container.html: Added.
+
2019-03-08 Truitt Savell <[email protected]>
(r242595) Layout Tests in imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/* are failing
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container-expected.txt (0 => 242661)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container-expected.txt 2019-03-08 23:51:05 UTC (rev 242661)
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is not shown below.
+
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container.html (0 => 242661)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container.html 2019-03-08 23:51:05 UTC (rev 242661)
@@ -0,0 +1,66 @@
+<html>
+<head>
+<title>This tests the case when we've got all the renderers constructed before they become visible and the container is not clickable.</title>
+<script src=""
+<style>
+#tapthis {
+ width: 400px;
+ height: 400px;
+ border: 1px solid green;
+}
+
+#becomesVisible {
+ max-height: 0px;
+ width: 100px;
+ height: 100px;
+ background-color: green;
+ overflow: hidden;
+}
+
+#becomesVisibleChild {
+ width: 50px;
+ height: 50px;
+ 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 not shown below.</div>
+<div id=becomesVisible><div id=becomesVisibleChild></div></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mouseover", function( event ) {
+ becomesVisible.style.maxHeight = "100px";
+ document.body.offsetHeight;
+ if (window.testRunner)
+ testRunner.notifyDone();
+}, false);
+
+becomesVisibleChild.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked";
+}, false);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (242660 => 242661)
--- trunk/Source/WebCore/ChangeLog 2019-03-08 23:31:38 UTC (rev 242660)
+++ trunk/Source/WebCore/ChangeLog 2019-03-08 23:51:05 UTC (rev 242661)
@@ -1,5 +1,30 @@
2019-03-08 Zalan Bujtas <[email protected]>
+ [ContentChangeObserver] Expand "isConsideredClickable" to descendants
+ https://bugs.webkit.org/show_bug.cgi?id=195478
+ <rdar://problem/48724935>
+
+ Reviewed by Simon Fraser.
+
+ In StyleChangeScope we try to figure out whether newly visible content should stick (menu panes etc) by checking if it is clickable.
+ This works fine as long as all the visible elements are gaining new renderers through this style update processs.
+ However when an element becomes visible by a change other than display: (not)none, it's not sufficient to just check the element itself,
+ since it might not respond to click at all, while its descendants do.
+ A concrete example is a max-height value change on usps.com, where the max-height is on a container (menu pane).
+ This container itself is not clickable while most of its children are (menu items).
+
+ Test: fast/events/touch/ios/content-observation/clickable-content-is-inside-a-container.html
+
+ * page/ios/ContentChangeObserver.cpp:
+ (WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
+ (WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
+ (WebCore::ContentChangeObserver::StyleChangeScope::isConsideredHidden const):
+ (WebCore::ContentChangeObserver::StyleChangeScope::isConsideredClickable const):
+ (WebCore::isConsideredHidden): Deleted.
+ * page/ios/ContentChangeObserver.h:
+
+2019-03-08 Zalan Bujtas <[email protected]>
+
[ContentChangeObserver] Cleanup adjustObservedState
https://bugs.webkit.org/show_bug.cgi?id=195470
<rdar://problem/48717823>
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (242660 => 242661)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-03-08 23:31:38 UTC (rev 242660)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-03-08 23:51:05 UTC (rev 242661)
@@ -33,6 +33,7 @@
#include "Logging.h"
#include "NodeRenderStyle.h"
#include "Page.h"
+#include "RenderDescendantIterator.h"
#include "Settings.h"
namespace WebCore {
@@ -295,12 +296,31 @@
}
}
-static bool isConsideredHidden(const Element& element)
+ContentChangeObserver::StyleChangeScope::StyleChangeScope(Document& document, const Element& element)
+ : m_contentChangeObserver(document.contentChangeObserver())
+ , m_element(element)
+ , m_hadRenderer(element.renderer())
{
- if (!element.renderStyle())
+ if (m_contentChangeObserver.isObservingContentChanges() && !m_contentChangeObserver.hasVisibleChangeState())
+ m_wasHidden = isConsideredHidden();
+}
+
+ContentChangeObserver::StyleChangeScope::~StyleChangeScope()
+{
+ auto changedFromHiddenToVisible = [&] {
+ return m_wasHidden && !isConsideredHidden();
+ };
+
+ if (changedFromHiddenToVisible() && isConsideredClickable())
+ m_contentChangeObserver.contentVisibilityDidChange();
+}
+
+bool ContentChangeObserver::StyleChangeScope::isConsideredHidden() const
+{
+ if (!m_element.renderStyle())
return true;
- auto& style = *element.renderStyle();
+ auto& style = *m_element.renderStyle();
if (style.display() == DisplayType::None)
return true;
@@ -329,31 +349,23 @@
return false;
}
-ContentChangeObserver::StyleChangeScope::StyleChangeScope(Document& document, const Element& element)
- : m_contentChangeObserver(document.contentChangeObserver())
- , m_element(element)
+bool ContentChangeObserver::StyleChangeScope::isConsideredClickable() const
{
- auto qualifiesForVisibilityCheck = [&] {
- if (m_element.isInUserAgentShadowTree())
- return false;
- if (!const_cast<Element&>(m_element).willRespondToMouseClickEvents())
- return false;
+ 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 needsObserving = m_contentChangeObserver.isObservingContentChanges() && !m_contentChangeObserver.hasVisibleChangeState() && qualifiesForVisibilityCheck();
- if (needsObserving)
- m_wasHidden = isConsideredHidden(m_element);
+ // 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())) {
+ if (descendant.element()->willRespondToMouseClickEvents())
+ return true;
+ }
+ return false;
}
-ContentChangeObserver::StyleChangeScope::~StyleChangeScope()
-{
- if (!m_wasHidden || isConsideredHidden(m_element))
- return;
-
- m_contentChangeObserver.contentVisibilityDidChange();
-}
-
#if ENABLE(TOUCH_EVENTS)
ContentChangeObserver::TouchEventScope::TouchEventScope(Document& document, PlatformEvent::Type eventType)
: m_contentChangeObserver(document.contentChangeObserver())
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.h (242660 => 242661)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2019-03-08 23:31:38 UTC (rev 242660)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2019-03-08 23:51:05 UTC (rev 242661)
@@ -58,9 +58,13 @@
~StyleChangeScope();
private:
+ bool isConsideredHidden() const;
+ bool isConsideredClickable() const;
+
ContentChangeObserver& m_contentChangeObserver;
const Element& m_element;
bool m_wasHidden { false };
+ bool m_hadRenderer { false };
};
class TouchEventScope {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
