Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Navigation within iframe doesn't exit fullscreen for parent iframe el…
…ement

https://bugs.webkit.org/show_bug.cgi?id=251896
rdar://104393093

Reviewed by Chris Dumez.

This was because the exiting document was already detached from the Frame object, causing it to fail to find the top document.
To fix this, we store the top document when initializing FullscreenManager.

* LayoutTests/fullscreen/fullscreen-iframe-navigation-expected.html: Added.
* LayoutTests/fullscreen/fullscreen-iframe-navigation.html: Added.
* LayoutTests/fullscreen/resources/fullscreen-iframe-navigation-target.html: Added.
* LayoutTests/platform/mac-wk1/TestExpectations:
* Source/WebCore/dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::cancelFullscreen):
(WebCore::documentsToUnfullscreen):
(WebCore::FullscreenManager::exitFullscreen):
(WebCore::FullscreenManager::finishExitFullscreen):
(WebCore::FullscreenManager::didExitFullscreen):
* Source/WebCore/dom/FullscreenManager.h:

Canonical link: https://commits.webkit.org/260024@main
  • Loading branch information
nt1m committed Feb 8, 2023
1 parent 0f4a54d commit 9215d7d
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 14 deletions.
@@ -0,0 +1,7 @@
<!DOCTYPE html>
<html>
<meta charset="UTF-8">
<body>
<iframe id="iframe" src="./resources/fullscreen-iframe-navigation-target.html"></iframe>
</body>
</html>
42 changes: 42 additions & 0 deletions LayoutTests/fullscreen/fullscreen-iframe-navigation.html
@@ -0,0 +1,42 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<meta charset="UTF-8">
<style>
.failed {
background-color: red;
}
</style>
</head>
<body>
<iframe id="iframe" srcdoc="<button onclick='document.documentElement.requestFullscreen().then(() => (window.location.href=`./resources/fullscreen-iframe-navigation-target.html`))'>Enter fullscreen and navigate</button>"></iframe>
<script src="../imported/w3c/web-platform-tests/resources/testdriver.js"></script>
<script src="../imported/w3c/web-platform-tests/resources/testdriver-vendor.js"></script>
<script>
addEventListener("load", async () => {
await test_driver.bless("fullscreen", null, iframe.contentWindow);
await new Promise(resolve => {
addEventListener("fullscreenchange", resolve, { once: true });
iframe.contentDocument.documentElement.requestFullscreen();
});
if (!document.fullscreenElement)
document.body.className = "failed";
await Promise.all([
new Promise(resolve => {
addEventListener("message", (e) => {
if (e.data == "loaded")
resolve();
}, { once: true });
iframe.contentWindow.location.href = "./resources/fullscreen-iframe-navigation-target.html";
}),
new Promise(resolve => {
addEventListener("fullscreenchange", resolve, { once: true });
})
]);
if (document.fullscreenElement)
document.body.className = "failed";
document.documentElement.classList.remove("reftest-wait");
});
</script>
</body>
</html>
@@ -0,0 +1,12 @@
<!DOCTYPE html>
<html>
<body>
<p>Successfully navigated, test passes if the iframe does not take the full content area size.</p>
<script>
addEventListener("DOMContentLoaded", () => {
if (parent)
parent.postMessage("loaded", "*");
}, { once: true });
</script>
</body>
</html>
3 changes: 3 additions & 0 deletions LayoutTests/platform/mac-wk1/TestExpectations
Expand Up @@ -2449,3 +2449,6 @@ imported/w3c/web-platform-tests/geolocation-API/permission.https.html [ WontFix

# Per-process limit is only for WK2
http/tests/websocket/tests/hybi/multiple-connections-limit.html [ Skip ]

# Displays blank on WK1
fullscreen/fullscreen-iframe-navigation.html [ Skip ]
29 changes: 16 additions & 13 deletions Source/WebCore/dom/FullscreenManager.cpp
Expand Up @@ -51,7 +51,7 @@ namespace WebCore {
using namespace HTMLNames;

FullscreenManager::FullscreenManager(Document& document)
: m_document { document }
: m_document(document)
#if !RELEASE_LOG_DISABLED
, m_logIdentifier(LoggerHelper::uniqueLogIdentifier())
#endif
Expand Down Expand Up @@ -155,6 +155,9 @@ void FullscreenManager::requestFullscreenForElement(Ref<Element>&& element, RefP

m_pendingFullscreenElement = RefPtr { element.ptr() };

// We cache the top document here, so we still have the correct one when we exit fullscreen after navigation.
m_topDocument = document().topDocument();

m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, weakThis = WeakPtr { *this }, element = WTFMove(element), promise = WTFMove(promise), checkType, hasKeyboardAccess, failedPreflights, identifier] () mutable {
if (!weakThis) {
if (promise)
Expand Down Expand Up @@ -261,8 +264,8 @@ void FullscreenManager::cancelFullscreen()
// is defined as:
// "To fully exit fullscreen act as if the exitFullscreen() method was invoked on the top-level browsing
// context's document and subsequently empty that document's fullscreen element stack."
Document& topDocument = document().topDocument();
if (!topDocument.fullscreenManager().fullscreenElement()) {
Ref topDocument = this->topDocument();
if (!topDocument->fullscreenManager().fullscreenElement()) {
// If there is a pending fullscreen element but no top document fullscreen element,
// there is a pending task in enterFullscreen(). Cause it to cancel and fire an error
// by clearing the pending fullscreen element.
Expand All @@ -279,14 +282,14 @@ void FullscreenManager::cancelFullscreen()

m_pendingExitFullscreen = true;

m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, &topDocument, identifier = LOGIDENTIFIER] {
if (!topDocument.page()) {
m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, topDocument = WTFMove(topDocument), identifier = LOGIDENTIFIER] {
if (!topDocument->page()) {
INFO_LOG(identifier, "Top document has no page.");
return;
}

// This triggers finishExitFullscreen with ExitMode::Resize, which fully exits the document.
if (auto* fullscreenElement = topDocument.fullscreenManager().fullscreenElement())
if (auto* fullscreenElement = topDocument->fullscreenManager().fullscreenElement())
page()->chrome().client().exitFullScreenForElement(fullscreenElement);
else
INFO_LOG(identifier, "Top document has no fullscreen element");
Expand Down Expand Up @@ -317,22 +320,22 @@ void FullscreenManager::exitFullscreen(RefPtr<DeferredPromise>&& promise)
{
INFO_LOG(LOGIDENTIFIER);

auto* exitingDocument = &document();
Ref exitingDocument = document();
auto mode = ExitMode::NoResize;
auto exitDocuments = documentsToUnfullscreen(*exitingDocument);
auto& topDocument = this->topDocument();
auto exitDocuments = documentsToUnfullscreen(exitingDocument);
Ref topDocument = this->topDocument();

bool exitsTopDocument = exitDocuments.containsIf([&](auto& document) {
return document.ptr() == &topDocument;
return document.ptr() == topDocument.ptr();
});
if (exitsTopDocument && topDocument.fullscreenManager().isSimpleFullscreenDocument()) {
if (exitsTopDocument && topDocument->fullscreenManager().isSimpleFullscreenDocument()) {
mode = ExitMode::Resize;
exitingDocument = &topDocument;
exitingDocument = topDocument;
}

auto element = exitingDocument->fullscreenManager().fullscreenElement();
if (element && !element->isConnected())
addDocumentToFullscreenChangeEventQueue(*exitingDocument);
addDocumentToFullscreenChangeEventQueue(exitingDocument);

m_pendingExitFullscreen = true;

Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/dom/FullscreenManager.h
Expand Up @@ -47,7 +47,6 @@ class FullscreenManager final : public CanMakeWeakPtr<FullscreenManager> {

Document& document() { return m_document; }
const Document& document() const { return m_document; }
Document& topDocument() const { return m_document.topDocument(); }
Page* page() const { return m_document.page(); }
Frame* frame() const { return m_document.frame(); }
Element* documentElement() const { return m_document.documentElement(); }
Expand Down Expand Up @@ -108,7 +107,10 @@ class FullscreenManager final : public CanMakeWeakPtr<FullscreenManager> {
WTFLogChannel& logChannel() const;
#endif

Document& topDocument() { return m_topDocument ? *m_topDocument : document().topDocument(); }

Document& m_document;
WeakPtr<Document, WeakPtrImplWithEventTargetData> m_topDocument;

RefPtr<Element> fullscreenOrPendingElement() const { return m_fullscreenElement ? m_fullscreenElement : m_pendingFullscreenElement; }

Expand Down

0 comments on commit 9215d7d

Please sign in to comment.