Skip to content

Commit 182bb25

Browse files
vinhillvinhill
committed
Bug 1858562 - Part 4: Browser Chrome Document Picture-in-Picture adjustments. r=mconley,smaug,sthompson,dom-core,webidl,sessionstore-reviewers,dao,urlbar-reviewers
Implement a basic UI/UX for Document Picture-in-Picture. - The opener URI is shown in the PiP window's location bar. - Fullscreen via F11 and session restore are blocked for PiP windows. Differential Revision: https://phabricator.services.mozilla.com/D267248
1 parent 8bf1985 commit 182bb25

File tree

13 files changed

+230
-15
lines changed

13 files changed

+230
-15
lines changed

‎browser/base/content/browser.js‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2215,16 +2215,19 @@ var XULBrowserWindow = {
22152215
aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SESSION_STORE
22162216
);
22172217

2218-
// We want to update the popup visibility if we received this notification
2219-
// via simulated locationchange events such as switching between tabs, however
2220-
// if this is a document navigation then PopupNotifications will be updated
2221-
// via TabsProgressListener.onLocationChange and we do not want it called twice
2222-
gURLBar.setURI({
2223-
uri: aLocationURI,
2224-
dueToTabSwitch: aIsSimulated,
2225-
dueToSessionRestore: isSessionRestore,
2226-
isSameDocument,
2227-
});
2218+
// Don't update URL for document PiP window as it shows its opener url
2219+
if (!window.browsingContext.isDocumentPiP) {
2220+
// We want to update the popup visibility if we received this notification
2221+
// via simulated locationchange events such as switching between tabs, however
2222+
// if this is a document navigation then PopupNotifications will be updated
2223+
// via TabsProgressListener.onLocationChange and we do not want it called twice
2224+
gURLBar.setURI({
2225+
uri: aLocationURI,
2226+
dueToTabSwitch: aIsSimulated,
2227+
dueToSessionRestore: isSessionRestore,
2228+
isSameDocument,
2229+
});
2230+
}
22282231

22292232
BookmarkingUI.onLocationChange();
22302233
// If we've actually changed document, update the toolbar visibility.
@@ -2542,6 +2545,13 @@ var XULBrowserWindow = {
25422545
aState |= Ci.nsIWebProgressListener.STATE_IDENTITY_ASSOCIATED;
25432546
}
25442547

2548+
if (window.browsingContext.isDocumentPiP) {
2549+
gURLBar.setURI({
2550+
uri,
2551+
isSameDocument: true,
2552+
});
2553+
}
2554+
25452555
try {
25462556
uri = Services.io.createExposableURI(uri);
25472557
} catch (e) {}

‎browser/components/sessionstore/SessionStore.sys.mjs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2260,6 +2260,12 @@ var SessionStoreInternal = {
22602260
* Window reference
22612261
*/
22622262
onBeforeBrowserWindowShown(aWindow) {
2263+
// Do not track Document Picture-in-Picture windows since these are
2264+
// ephemeral and tied to a specific tab's browser document.
2265+
if (aWindow.browsingContext.isDocumentPiP) {
2266+
return;
2267+
}
2268+
22632269
// Register the window.
22642270
this.onLoad(aWindow);
22652271

‎browser/components/urlbar/content/UrlbarInput.mjs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,14 @@ export class UrlbarInput extends HTMLElement {
717717
"Cannot set URI for UrlbarInput that is not an address bar"
718718
);
719719
}
720+
if (
721+
this.window.browsingContext.isDocumentPiP &&
722+
uri.spec.startsWith("about:blank")
723+
) {
724+
// If this is a Document PiP, its url will be about:blank while
725+
// the opener will be a secure context, i.e. no about:blank
726+
throw new Error("Document PiP should show its opener URL");
727+
}
720728
// We only need to update the searchModeUI on tab switch conditionally
721729
// as we only persist searchMode with ScotchBonnet enabled.
722730
if (

‎dom/base/nsGlobalWindowOuter.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4223,6 +4223,12 @@ nsresult nsGlobalWindowOuter::SetFullscreenInternal(FullscreenReason aReason,
42234223
return NS_OK;
42244224
}
42254225

4226+
// Element.requestFullscreen() is already blocked, but also block
4227+
// fullscreening for other callers, especially the chrome window.
4228+
if (GetBrowsingContext()->Top()->GetIsDocumentPiP()) {
4229+
return NS_OK;
4230+
}
4231+
42264232
// SetFullscreen needs to be called on the root window, so get that
42274233
// via the DocShell tree, and if we are not already the root,
42284234
// call SetFullscreen on that window instead.

‎dom/chrome-webidl/BrowsingContext.webidl‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,12 @@ interface BrowsingContext {
301301
undefined resetNavigationRateLimit();
302302

303303
readonly attribute long childOffset;
304+
305+
// https://wicg.github.io/document-picture-in-picture/
306+
// This is true both for the top-level BC of the content and chrome window
307+
// of a Document Picture-in-Picture window.
308+
[BinaryName="GetIsDocumentPiP"]
309+
readonly attribute boolean isDocumentPiP;
304310
};
305311

306312
BrowsingContext includes LoadContextMixin;

‎dom/documentpip/moz.build‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ UNIFIED_SOURCES += [
1313
"DocumentPictureInPicture.cpp",
1414
]
1515

16+
BROWSER_CHROME_MANIFESTS += ["tests/browser/browser.toml"]
17+
1618
include("/ipc/chromium/chromium-config.mozbuild")
1719

1820
FINAL_LIBRARY = "xul"

‎dom/documentpip/tests/browser/browser.toml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,12 @@
22
support-files = ["head.js"]
33
prefs = ["dom.documentpip.enabled=true"]
44

5+
["browser_parent_has_pip_flag.js"]
6+
57
["browser_pip_closes_once.js"]
8+
9+
["browser_pip_fullscreen_disallowed.js"]
10+
11+
["browser_pip_ui.js"]
12+
13+
["browser_sessionstore_ignore_dpip.js"]

‎dom/documentpip/tests/browser/browser_parent_has_pip_flag.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/* Any copyright is dedicated to the Public Domain.
2+
http://creativecommons.org/publicdomain/zero/1.0/ */
3+
4+
"use strict";
5+
6+
add_task(async function check_chrome_window_has_pip_flag() {
7+
const [tab, chromePiP] = await newTabWithPiP();
8+
9+
is(
10+
chromePiP.location.href,
11+
"chrome://browser/content/browser.xhtml",
12+
"Got the chrome window of the PiP"
13+
);
14+
ok(chromePiP.browsingContext.isDocumentPiP, "Chrome window has pip flag set");
15+
16+
// Cleanup.
17+
await BrowserTestUtils.closeWindow(chromePiP);
18+
BrowserTestUtils.removeTab(tab);
19+
});

‎dom/documentpip/tests/browser/browser_pip_closes_once.js‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,31 @@
1313
add_task(async function closing_pip_sends_exactly_one_DOMWindowClosed() {
1414
const [tab, chromePiP] = await newTabWithPiP();
1515

16-
let closeCount = 0;
17-
chromePiP.addEventListener("DOMWindowClose", () => closeCount++);
16+
// Note: Counting DOMWindowClose in the parent process isn't the same.
17+
// - The parent might have multiple, i.e. for closing tab and native window
18+
// - Sending the second event up to the parent might fail if closing has progressed far enough
19+
await SpecialPowers.spawn(chromePiP.gBrowser.selectedBrowser, [], () => {
20+
content.opener.closeCount = 0;
21+
SpecialPowers.addChromeEventListener(
22+
"DOMWindowClose",
23+
() => content.opener.closeCount++,
24+
true,
25+
false
26+
);
27+
});
1828

29+
// close PiP
1930
await SpecialPowers.spawn(tab.linkedBrowser, [], () => {
2031
content.documentPictureInPicture.window.close();
2132
});
2233
await BrowserTestUtils.windowClosed(chromePiP);
2334

35+
const closeCount = await SpecialPowers.spawn(
36+
tab.linkedBrowser,
37+
[],
38+
() => content.closeCount
39+
);
2440
is(closeCount, 1, "Received a single DOMWindowClosed");
41+
2542
BrowserTestUtils.removeTab(tab);
2643
});

‎dom/documentpip/tests/browser/browser_pip_fullscreen_disallowed.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/* Any copyright is dedicated to the Public Domain.
2+
http://creativecommons.org/publicdomain/zero/1.0/ */
3+
4+
"use strict";
5+
6+
add_task(async function check_chrome_window_of_pip_cannot_fullscreen() {
7+
const [tab, chromePiP] = await newTabWithPiP();
8+
9+
// Sanity check: VK_F11 will set window.fullScreen via View:Fullscreen command
10+
const fullScreenEntered = BrowserTestUtils.waitForEvent(window, "fullscreen");
11+
window.fullScreen = true;
12+
ok(window.fullScreen, "FS works as expected on window");
13+
await fullScreenEntered;
14+
ok(true, "Got fullscreen event");
15+
16+
const fullScreenExited = BrowserTestUtils.waitForEvent(window, "fullscreen");
17+
window.fullScreen = false;
18+
ok(!window.fullScreen, "FS works as expected on window");
19+
await fullScreenExited;
20+
ok(true, "Got fullscreen event");
21+
22+
// Check PiP cannot be fullscreened by chrome
23+
is(
24+
chromePiP.location.href,
25+
"chrome://browser/content/browser.xhtml",
26+
"Got the chrome window of the PiP"
27+
);
28+
chromePiP.fullScreen = true;
29+
ok(!chromePiP.fullScreen, "Didn't enter fullscreen on PiP");
30+
31+
// Cleanup.
32+
await BrowserTestUtils.closeWindow(chromePiP);
33+
BrowserTestUtils.removeTab(tab);
34+
});

0 commit comments

Comments
 (0)