- Revision
- 287588
- Author
- [email protected]
- Date
- 2022-01-04 13:58:51 -0800 (Tue, 04 Jan 2022)
Log Message
ModalContainerObserver should search for text in subframes
https://bugs.webkit.org/show_bug.cgi?id=234446
rdar://86897770
Reviewed by Tim Horton.
Source/WebCore:
Adds support for detecting modal containers when the search term is embedded inside a subframe in the top
document. To do this, we make a few adjustments to ModalContainerObserver, detailed in the comments below. This
supports both scenarios in which (1) a subframe that has not yet loaded is added to the top document and then
later gains occurrences of the search term, and (2) a subframe that already contains the search term in rendered
text is made visible inside the top document.
Test: ModalContainerObservation.ModalContainerInSubframe
* page/ModalContainerObserver.cpp:
(WebCore::ModalContainerObserver::isNeededFor):
Install a ModalContainerObserver for child documents in the case where the parent document already has a modal
container observer that is waiting for this frame to finish its first visually non-empty layout (see the comment
in `ModalContainerObserver::updateModalContainerIfNeeded()`, below).
(WebCore::containsMatchingText):
Pull out logic for searching all text renderer descendants underneath a given renderer into a separate helper
function.
(WebCore::ModalContainerObserver::searchForModalContainerOnBehalfOfFrameOwnerIfNeeded):
Used for a ModalContainerObserver in a child frame to notify the ModalContainerObserver in the parent document
when the child document has finished layout. If this child document's owner element is one of the elements in
`m_frameOwnersAndContainersToSearchAgain`, we invalidate the corresponding viewport-constrained container that
we've previously searched (i.e. by removing it from `m_elementsToIgnoreWhenSearching`) and re-search the
container.
(WebCore::ModalContainerObserver::updateModalContainerIfNeeded):
Make a couple of adjustments:
- For modal container observers that are not in the top document: delegate the modal container update out
to the parent document's modal container observer, by calling the helper method above.
- For modal container observers that are in the top document: add logic to search content in subframes
inside of viewport-constrained containers as a fallback in the case where the search term does not
appear in content at the top document.
(WebCore::ModalContainerObserver::setContainer):
Pull out logic for tracking the currently detected modal container into a separate helper method that takes
the modal container element, as well as the frame owner element where we observed the search term (or null if
the search term was observed in the top document).
(WebCore::ModalContainerObserver::container const):
(WebCore::ModalContainerObserver::frameOwnerForControls const):
Replace `m_container` with a `std::pair` containing the current modal container (i.e. what is currently just
`m_container`, as well as a weak pointer to the associated frame owner element where we discovered the search
term). We use a `std::pair` here because these two elements should always be set and cleared out in tandem.
(WebCore::ModalContainerObserver::collectClickableElementsTimerFired):
(WebCore::ModalContainerObserver::revealModalContainer):
Replace uses of `m_container` with the `container()` method.
(WebCore::ModalContainerObserver::collectClickableElements):
If a frame owner element is set, use the subframe's content document element as the container for collecting
clickable controls instead of using the actual modal container element in the top document.
* page/ModalContainerObserver.h:
(WebCore::ModalContainerObserver::shouldHide const):
Use `container()` instead of `m_container`.
Tools:
Add an API test to exercise the change. See WebCore/ChangLog for more details.
* TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (287587 => 287588)
--- trunk/Source/WebCore/ChangeLog 2022-01-04 21:44:57 UTC (rev 287587)
+++ trunk/Source/WebCore/ChangeLog 2022-01-04 21:58:51 UTC (rev 287588)
@@ -1,3 +1,78 @@
+2022-01-04 Wenson Hsieh <[email protected]>
+
+ ModalContainerObserver should search for text in subframes
+ https://bugs.webkit.org/show_bug.cgi?id=234446
+ rdar://86897770
+
+ Reviewed by Tim Horton.
+
+ Adds support for detecting modal containers when the search term is embedded inside a subframe in the top
+ document. To do this, we make a few adjustments to ModalContainerObserver, detailed in the comments below. This
+ supports both scenarios in which (1) a subframe that has not yet loaded is added to the top document and then
+ later gains occurrences of the search term, and (2) a subframe that already contains the search term in rendered
+ text is made visible inside the top document.
+
+ Test: ModalContainerObservation.ModalContainerInSubframe
+
+ * page/ModalContainerObserver.cpp:
+ (WebCore::ModalContainerObserver::isNeededFor):
+
+ Install a ModalContainerObserver for child documents in the case where the parent document already has a modal
+ container observer that is waiting for this frame to finish its first visually non-empty layout (see the comment
+ in `ModalContainerObserver::updateModalContainerIfNeeded()`, below).
+
+ (WebCore::containsMatchingText):
+
+ Pull out logic for searching all text renderer descendants underneath a given renderer into a separate helper
+ function.
+
+ (WebCore::ModalContainerObserver::searchForModalContainerOnBehalfOfFrameOwnerIfNeeded):
+
+ Used for a ModalContainerObserver in a child frame to notify the ModalContainerObserver in the parent document
+ when the child document has finished layout. If this child document's owner element is one of the elements in
+ `m_frameOwnersAndContainersToSearchAgain`, we invalidate the corresponding viewport-constrained container that
+ we've previously searched (i.e. by removing it from `m_elementsToIgnoreWhenSearching`) and re-search the
+ container.
+
+ (WebCore::ModalContainerObserver::updateModalContainerIfNeeded):
+
+ Make a couple of adjustments:
+
+ - For modal container observers that are not in the top document: delegate the modal container update out
+ to the parent document's modal container observer, by calling the helper method above.
+
+ - For modal container observers that are in the top document: add logic to search content in subframes
+ inside of viewport-constrained containers as a fallback in the case where the search term does not
+ appear in content at the top document.
+
+ (WebCore::ModalContainerObserver::setContainer):
+
+ Pull out logic for tracking the currently detected modal container into a separate helper method that takes
+ the modal container element, as well as the frame owner element where we observed the search term (or null if
+ the search term was observed in the top document).
+
+ (WebCore::ModalContainerObserver::container const):
+ (WebCore::ModalContainerObserver::frameOwnerForControls const):
+
+ Replace `m_container` with a `std::pair` containing the current modal container (i.e. what is currently just
+ `m_container`, as well as a weak pointer to the associated frame owner element where we discovered the search
+ term). We use a `std::pair` here because these two elements should always be set and cleared out in tandem.
+
+ (WebCore::ModalContainerObserver::collectClickableElementsTimerFired):
+ (WebCore::ModalContainerObserver::revealModalContainer):
+
+ Replace uses of `m_container` with the `container()` method.
+
+ (WebCore::ModalContainerObserver::collectClickableElements):
+
+ If a frame owner element is set, use the subframe's content document element as the container for collecting
+ clickable controls instead of using the actual modal container element in the top document.
+
+ * page/ModalContainerObserver.h:
+ (WebCore::ModalContainerObserver::shouldHide const):
+
+ Use `container()` instead of `m_container`.
+
2022-01-04 Jer Noble <[email protected]>
[Cocoa] Hang in AVTrackPrivateAVFObjCImpl::bitrate()
Modified: trunk/Source/WebCore/page/ModalContainerObserver.cpp (287587 => 287588)
--- trunk/Source/WebCore/page/ModalContainerObserver.cpp 2022-01-04 21:44:57 UTC (rev 287587)
+++ trunk/Source/WebCore/page/ModalContainerObserver.cpp 2022-01-04 21:58:51 UTC (rev 287588)
@@ -60,18 +60,13 @@
bool ModalContainerObserver::isNeededFor(const Document& document)
{
- RefPtr documentLoader = document.loader();
- if (!documentLoader || documentLoader->modalContainerObservationPolicy() == ModalContainerObservationPolicy::Disabled)
+ RefPtr topDocumentLoader = document.topDocument().loader();
+ if (!topDocumentLoader || topDocumentLoader->modalContainerObservationPolicy() == ModalContainerObservationPolicy::Disabled)
return false;
- if (!document.url().protocolIsInHTTPFamily())
+ if (!document.topDocument().url().protocolIsInHTTPFamily())
return false;
- if (!document.isTopDocument()) {
- // FIXME (234446): Need to properly support subframes.
- return false;
- }
-
if (document.inDesignMode() || !is<HTMLDocument>(document))
return false;
@@ -83,6 +78,11 @@
if (!page || page->isEditable())
return false;
+ if (RefPtr owner = document.ownerElement()) {
+ auto observer = owner->document().modalContainerObserverIfExists();
+ return observer && observer->m_frameOwnersAndContainersToSearchAgain.contains(*owner);
+ }
+
return true;
}
@@ -145,14 +145,45 @@
return false;
}
+static bool containsMatchingText(RenderLayerModelObject& renderer, const AtomString& searchTerm)
+{
+ for (auto& textRenderer : descendantsOfType<RenderText>(renderer)) {
+ if (RefPtr textNode = textRenderer.textNode(); textNode && matchesSearchTerm(*textNode, searchTerm))
+ return !isInsideNavigationElement(*textNode);
+ }
+ return false;
+}
+
+void ModalContainerObserver::searchForModalContainerOnBehalfOfFrameOwnerIfNeeded(HTMLFrameOwnerElement& owner)
+{
+ auto containerToSearchAgain = m_frameOwnersAndContainersToSearchAgain.take(owner);
+ if (!containerToSearchAgain)
+ return;
+
+ if (!m_elementsToIgnoreWhenSearching.remove(*containerToSearchAgain))
+ return;
+
+ if (RefPtr view = owner.document().view())
+ updateModalContainerIfNeeded(*view);
+}
+
void ModalContainerObserver::updateModalContainerIfNeeded(const FrameView& view)
{
- if (m_container)
+ if (container())
return;
if (m_hasAttemptedToFulfillPolicy)
return;
+ if (RefPtr owner = view.frame().ownerElement()) {
+ if (auto parentObserver = owner->document().modalContainerObserverIfExists())
+ parentObserver->searchForModalContainerOnBehalfOfFrameOwnerIfNeeded(*owner);
+ return;
+ }
+
+ if (!view.frame().isMainFrame())
+ return;
+
if (!view.hasViewportConstrainedObjects())
return;
@@ -173,6 +204,9 @@
if (renderer.isDocumentElementRenderer())
continue;
+ if (renderer.style().visibility() == Visibility::Hidden)
+ continue;
+
RefPtr element = renderer.element();
if (!element || is<HTMLBodyElement>(*element) || element->isDocumentNode())
continue;
@@ -180,20 +214,54 @@
if (!m_elementsToIgnoreWhenSearching.add(*element).isNewEntry)
continue;
- for (auto& textRenderer : descendantsOfType<RenderText>(renderer)) {
- if (RefPtr textNode = textRenderer.textNode(); textNode && matchesSearchTerm(*textNode, searchTerm)) {
- if (isInsideNavigationElement(*textNode))
- break;
+ if (containsMatchingText(renderer, searchTerm)) {
+ setContainer(*element);
+ return;
+ }
- m_container = element.get();
- element->invalidateStyle();
- scheduleClickableElementCollection();
+ for (auto& frameOwner : descendantsOfType<HTMLFrameOwnerElement>(*element)) {
+ RefPtr contentFrame = frameOwner.contentFrame();
+ if (!contentFrame)
+ continue;
+
+ auto renderView = contentFrame->contentRenderer();
+ if (!renderView)
+ continue;
+
+ if (containsMatchingText(*renderView, searchTerm)) {
+ setContainer(*element, &frameOwner);
return;
}
+
+ if (auto frameView = contentFrame->view(); frameView && !frameView->isVisuallyNonEmpty()) {
+ // If the subframe content has not become visually non-empty yet, search the subframe again later.
+ m_frameOwnersAndContainersToSearchAgain.add(frameOwner, *element);
+ }
}
}
}
+void ModalContainerObserver::setContainer(Element& newContainer, HTMLFrameOwnerElement* frameOwner)
+{
+ if (container())
+ container()->invalidateStyle();
+
+ m_containerAndFrameOwnerForControls = { { newContainer }, { frameOwner } };
+
+ newContainer.invalidateStyle();
+ scheduleClickableElementCollection();
+}
+
+Element* ModalContainerObserver::container() const
+{
+ return m_containerAndFrameOwnerForControls.first.get();
+}
+
+HTMLFrameOwnerElement* ModalContainerObserver::frameOwnerForControls() const
+{
+ return m_containerAndFrameOwnerForControls.second.get();
+}
+
static bool isClickableControl(const HTMLElement& element)
{
if (element.isActuallyDisabled())
@@ -327,15 +395,15 @@
void ModalContainerObserver::collectClickableElementsTimerFired()
{
- if (!m_container)
+ if (!container())
return;
- m_container->document().eventLoop().queueTask(TaskSource::InternalAsyncTask, [observer = this, decisionScope = ModalContainerPolicyDecisionScope { m_container->document() }]() mutable {
+ container()->document().eventLoop().queueTask(TaskSource::InternalAsyncTask, [observer = this, decisionScope = ModalContainerPolicyDecisionScope { container()->document() }]() mutable {
RefPtr document = decisionScope.document();
if (!document)
return;
- if (observer != document->modalContainerObserverIfExists() || !observer->m_container.get()) {
+ if (observer != document->modalContainerObserverIfExists() || !observer->container()) {
ASSERT_NOT_REACHED();
return;
}
@@ -466,13 +534,14 @@
void ModalContainerObserver::revealModalContainer()
{
- if (auto container = std::exchange(m_container, { }))
+ auto [container, frameOwner] = std::exchange(m_containerAndFrameOwnerForControls, { });
+ if (container)
container->invalidateStyle();
}
std::pair<Vector<WeakPtr<HTMLElement>>, Vector<String>> ModalContainerObserver::collectClickableElements()
{
- Ref container = *m_container;
+ Ref container = *this->container();
m_collectingClickableElements = true;
auto exitCollectClickableElementsScope = makeScopeExit([&] {
m_collectingClickableElements = false;
@@ -482,8 +551,23 @@
container->invalidateStyle();
container->document().updateLayoutIgnorePendingStylesheets();
+ auto containerForControls = ([&]() -> RefPtr<Element> {
+ auto frameOwner = frameOwnerForControls();
+ if (!frameOwner)
+ return container.ptr();
+
+ auto contentDocument = frameOwner->contentDocument();
+ if (!contentDocument)
+ return { };
+
+ return contentDocument->documentElement();
+ })();
+
+ if (!containerForControls)
+ return { };
+
Vector<Ref<HTMLElement>> clickableControls;
- for (auto& child : descendantsOfType<HTMLElement>(container)) {
+ for (auto& child : descendantsOfType<HTMLElement>(*containerForControls)) {
if (isClickableControl(child))
clickableControls.append(child);
}
Modified: trunk/Source/WebCore/page/ModalContainerObserver.h (287587 => 287588)
--- trunk/Source/WebCore/page/ModalContainerObserver.h 2022-01-04 21:44:57 UTC (rev 287587)
+++ trunk/Source/WebCore/page/ModalContainerObserver.h 2022-01-04 21:58:51 UTC (rev 287588)
@@ -28,6 +28,7 @@
#include "Timer.h"
#include <wtf/FastMalloc.h>
#include <wtf/Vector.h>
+#include <wtf/WeakHashMap.h>
#include <wtf/WeakHashSet.h>
#include <wtf/WeakPtr.h>
#include <wtf/text/AtomString.h>
@@ -39,6 +40,7 @@
class Element;
class FrameView;
class HTMLElement;
+class HTMLFrameOwnerElement;
class ModalContainerObserver {
WTF_MAKE_FAST_ALLOCATED;
@@ -59,11 +61,17 @@
void scheduleClickableElementCollection();
void collectClickableElementsTimerFired();
void revealModalContainer();
+ void setContainer(Element&, HTMLFrameOwnerElement* = nullptr);
+ void searchForModalContainerOnBehalfOfFrameOwnerIfNeeded(HTMLFrameOwnerElement&);
+ Element* container() const;
+ HTMLFrameOwnerElement* frameOwnerForControls() const;
+
std::pair<Vector<WeakPtr<HTMLElement>>, Vector<String>> collectClickableElements();
WeakHashSet<Element> m_elementsToIgnoreWhenSearching;
- WeakPtr<Element> m_container;
+ std::pair<WeakPtr<Element>, WeakPtr<HTMLFrameOwnerElement>> m_containerAndFrameOwnerForControls;
+ WeakHashMap<HTMLFrameOwnerElement, WeakPtr<Element>> m_frameOwnersAndContainersToSearchAgain;
AtomString m_overrideSearchTermForTesting;
Timer m_collectClickableElementsTimer;
bool m_collectingClickableElements { false };
@@ -77,7 +85,7 @@
inline bool ModalContainerObserver::shouldHide(const Element& element) const
{
- return m_container == &element && !m_collectingClickableElements;
+ return container() == &element && !m_collectingClickableElements;
}
} // namespace WebCore
Modified: trunk/Tools/ChangeLog (287587 => 287588)
--- trunk/Tools/ChangeLog 2022-01-04 21:44:57 UTC (rev 287587)
+++ trunk/Tools/ChangeLog 2022-01-04 21:58:51 UTC (rev 287588)
@@ -1,3 +1,16 @@
+2022-01-04 Wenson Hsieh <[email protected]>
+
+ ModalContainerObserver should search for text in subframes
+ https://bugs.webkit.org/show_bug.cgi?id=234446
+ rdar://86897770
+
+ Reviewed by Tim Horton.
+
+ Add an API test to exercise the change. See WebCore/ChangLog for more details.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm:
+ (TestWebKitAPI::TEST):
+
2022-01-04 Jonathan Bedard <[email protected]>
[git-webkit] Open closed pull-request when running pr (Follow-up fix)
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm (287587 => 287588)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm 2022-01-04 21:44:57 UTC (rev 287587)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm 2022-01-04 21:58:51 UTC (rev 287588)
@@ -288,4 +288,14 @@
EXPECT_NULL([webView lastModalContainerInfo]);
}
+TEST(ModalContainerObservation, ModalContainerInSubframe)
+{
+ auto webView = createModalContainerWebView();
+ [webView setDecision:_WKModalContainerDecisionHideAndIgnore];
+ [webView loadBundlePage:@"modal-container-custom"];
+ [webView evaluate:@"show(`<p>subframe test</p><iframe srcdoc='<h2>hello world</h2><button>YES</button>'></iframe>`)" andDecidePolicy:_WKModalContainerDecisionHideAndIgnore];
+ EXPECT_FALSE([[webView contentsAsString] containsString:@"subframe test"]);
+ EXPECT_EQ([webView lastModalContainerInfo].availableTypes, _WKModalContainerControlTypePositive);
+}
+
} // namespace TestWebKitAPI
