Title: [274864] trunk/Source/WebCore
- Revision
- 274864
- Author
- [email protected]
- Date
- 2021-03-23 06:39:03 -0700 (Tue, 23 Mar 2021)
Log Message
[ContentChangeObserver] Unable to view state details on CDC COVID map
https://bugs.webkit.org/show_bug.cgi?id=223620
<rdar://74284133>
Reviewed by Simon Fraser.
When the content change observer sees some visibility change, it checks if the newly visible content is actionable (e.g. something the user can click on).
A non-actionable content is considered less important than the action behind the "click". So while we trigger the hover state we immediately proceed with click as well.
(e.g. on youtube.com, hovering over the controls (settings, volume etc) brings up a (non-actionable) tooltip and the subsequent click triggers the associated action.
Now on iPadOS, it would require 2 taps on the mute/unmute button to actually mute/unmute the video, if we stopped at the hover state)
This patch implements a quirk for the CDC Covid map so that tapping on the map brings up the numbers dialog and we only submit the click event on the subsequent tap.
* page/Quirks.cpp:
(WebCore::Quirks::shouldTooltipPreventFromProceedingWithClick const):
* page/Quirks.h:
* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::isConsideredActionableContent const): moved the isConsideredClickable logic to a lambda function. Call it when we don't apply the quirk.
(WebCore::ContentChangeObserver::didFinishTransition):
(WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
(WebCore::isConsideredClickable): Deleted.
* page/ios/ContentChangeObserver.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (274863 => 274864)
--- trunk/Source/WebCore/ChangeLog 2021-03-23 13:37:46 UTC (rev 274863)
+++ trunk/Source/WebCore/ChangeLog 2021-03-23 13:39:03 UTC (rev 274864)
@@ -1,3 +1,28 @@
+2021-03-23 Zalan Bujtas <[email protected]>
+
+ [ContentChangeObserver] Unable to view state details on CDC COVID map
+ https://bugs.webkit.org/show_bug.cgi?id=223620
+ <rdar://74284133>
+
+ Reviewed by Simon Fraser.
+
+ When the content change observer sees some visibility change, it checks if the newly visible content is actionable (e.g. something the user can click on).
+ A non-actionable content is considered less important than the action behind the "click". So while we trigger the hover state we immediately proceed with click as well.
+ (e.g. on youtube.com, hovering over the controls (settings, volume etc) brings up a (non-actionable) tooltip and the subsequent click triggers the associated action.
+ Now on iPadOS, it would require 2 taps on the mute/unmute button to actually mute/unmute the video, if we stopped at the hover state)
+
+ This patch implements a quirk for the CDC Covid map so that tapping on the map brings up the numbers dialog and we only submit the click event on the subsequent tap.
+
+ * page/Quirks.cpp:
+ (WebCore::Quirks::shouldTooltipPreventFromProceedingWithClick const):
+ * page/Quirks.h:
+ * page/ios/ContentChangeObserver.cpp:
+ (WebCore::ContentChangeObserver::isConsideredActionableContent const): moved the isConsideredClickable logic to a lambda function. Call it when we don't apply the quirk.
+ (WebCore::ContentChangeObserver::didFinishTransition):
+ (WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
+ (WebCore::isConsideredClickable): Deleted.
+ * page/ios/ContentChangeObserver.h:
+
2021-03-23 Frédéric Wang <[email protected]>
Nullopt in DOMSelection::getRangeAt
Modified: trunk/Source/WebCore/page/Quirks.cpp (274863 => 274864)
--- trunk/Source/WebCore/page/Quirks.cpp 2021-03-23 13:37:46 UTC (rev 274863)
+++ trunk/Source/WebCore/page/Quirks.cpp 2021-03-23 13:39:03 UTC (rev 274864)
@@ -203,6 +203,16 @@
return host.endsWith(".youtube.com") || host == "youtube.com";
}
+bool Quirks::shouldTooltipPreventFromProceedingWithClick(const Element& element) const
+{
+ if (!needsQuirks())
+ return false;
+
+ if (!equalLettersIgnoringASCIICase(m_document->topDocument().url().host(), "covid.cdc.gov"))
+ return false;
+ return element.hasClass() && element.classNames().contains("tooltip");
+}
+
bool Quirks::needsMillisecondResolutionForHighResTimeStamp() const
{
if (!needsQuirks())
Modified: trunk/Source/WebCore/page/Quirks.h (274863 => 274864)
--- trunk/Source/WebCore/page/Quirks.h 2021-03-23 13:37:46 UTC (rev 274863)
+++ trunk/Source/WebCore/page/Quirks.h 2021-03-23 13:39:03 UTC (rev 274864)
@@ -76,6 +76,7 @@
bool needsInputModeNoneImplicitly(const HTMLElement&) const;
bool needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand() const;
bool shouldDisableContentChangeObserverTouchEventAdjustment() const;
+ bool shouldTooltipPreventFromProceedingWithClick(const Element&) const;
bool needsMillisecondResolutionForHighResTimeStamp() const;
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (274863 => 274864)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2021-03-23 13:37:46 UTC (rev 274863)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2021-03-23 13:39:03 UTC (rev 274864)
@@ -133,34 +133,38 @@
return true;
}
-enum class ElementHadRenderer { No, Yes };
-static bool isConsideredClickable(const Element& candidateElement, ElementHadRenderer hadRenderer)
+bool ContentChangeObserver::isConsideredActionableContent(const Element& candidateElement, ElementHadRenderer hadRenderer) const
{
- auto& element = const_cast<Element&>(candidateElement);
- if (element.isInUserAgentShadowTree())
- return false;
-
- if (is<HTMLIFrameElement>(element))
+ if (m_document.quirks().shouldTooltipPreventFromProceedingWithClick(candidateElement))
return true;
- if (is<HTMLImageElement>(element)) {
- // This is required to avoid HTMLImageElement's touch callout override logic. See rdar://problem/48937767.
- return element.Element::willRespondToMouseClickEvents();
- }
+ auto isConsideredClickable = [&] {
+ auto& element = const_cast<Element&>(candidateElement);
+ if (element.isInUserAgentShadowTree())
+ return false;
- bool hasRenderer = element.renderer();
- auto willRespondToMouseClickEvents = element.willRespondToMouseClickEvents();
- if (willRespondToMouseClickEvents || !hasRenderer || hadRenderer == ElementHadRenderer::No)
- return willRespondToMouseClickEvents;
+ if (is<HTMLIFrameElement>(element))
+ return true;
- // 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;
- if (descendant.element()->willRespondToMouseClickEvents())
- return true;
- }
- return false;
+ if (is<HTMLImageElement>(element)) {
+ // This is required to avoid HTMLImageElement's touch callout override logic. See rdar://problem/48937767.
+ return element.Element::willRespondToMouseClickEvents();
+ }
+ bool hasRenderer = element.renderer();
+ auto willRespondToMouseClickEvents = element.willRespondToMouseClickEvents();
+ if (willRespondToMouseClickEvents || !hasRenderer || hadRenderer == ElementHadRenderer::No)
+ return willRespondToMouseClickEvents;
+
+ // 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;
+ if (descendant.element()->willRespondToMouseClickEvents())
+ return true;
+ }
+ return false;
+ };
+ return isConsideredClickable();
}
ContentChangeObserver::ContentChangeObserver(Document& document)
@@ -247,7 +251,7 @@
return;
LOG_WITH_STREAM(ContentObservation, stream << "didFinishTransition: transition finished (" << &element << ").");
- // isConsideredClickable may trigger style update through Node::computeEditability. Let's adjust the state in the next runloop.
+ // isConsideredActionableContent may trigger style update through Node::computeEditability. Let's adjust the state in the next runloop.
callOnMainThread([weakThis = makeWeakPtr(*this), targetElement = makeWeakPtr(element)] {
if (!weakThis || !targetElement)
return;
@@ -255,7 +259,7 @@
weakThis->adjustObservedState(Event::EndedTransitionButFinalStyleIsNotDefiniteYet);
return;
}
- if (isConsideredClickable(*targetElement, ElementHadRenderer::Yes))
+ if (weakThis->isConsideredActionableContent(*targetElement, ElementHadRenderer::Yes))
weakThis->elementDidBecomeVisible(*targetElement);
weakThis->adjustObservedState(Event::CompletedTransition);
});
@@ -658,7 +662,7 @@
if (!m_wasHidden.hasValue())
return;
- if (!isConsideredClickable(m_element, m_hadRenderer ? ElementHadRenderer::Yes : ElementHadRenderer::No))
+ if (!m_contentChangeObserver.isConsideredActionableContent(m_element, m_hadRenderer ? ElementHadRenderer::Yes : ElementHadRenderer::No))
return;
auto wasVisible = !m_wasHidden.value();
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.h (274863 => 274864)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2021-03-23 13:37:46 UTC (rev 274863)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2021-03-23 13:39:03 UTC (rev 274864)
@@ -182,6 +182,9 @@
bool visibleRendererWasDestroyed(const Element& element) const { return m_elementsWithDestroyedVisibleRenderer.contains(&element); }
bool shouldObserveVisibilityChangeForElement(const Element&);
+ enum class ElementHadRenderer { No, Yes };
+ bool isConsideredActionableContent(const Element&, ElementHadRenderer) const;
+
enum class Event {
StartedTouchStartEventDispatching,
EndedTouchStartEventDispatching,
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
