Skip to content

Commit

Permalink
Bug 1753608 - Fix Session Restore fails to restore some features. r=Gijs
Browse files Browse the repository at this point in the history
* I overlooked that some `window.open` feature names are different from
  barprop names.
* Adding "resizable" will regress the maximize button prblem. But it was
  broken even before bug 1564738 and fixing it requires changes to session
  data. The current session data do not contain enough information to restore
  the maximize button state correctly. I'll file a follow-up bug about this.
* I renamed the test file because it is no longer limited to tab visibility.

Differential Revision: https://phabricator.services.mozilla.com/D137838
  • Loading branch information
vyv03354 committed Feb 4, 2022
1 parent 585a021 commit 9aff9db
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 55 deletions.
1 change: 0 additions & 1 deletion browser/base/content/test/tabs/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ skip-if =
[browser_tab_label_picture_in_picture.js]
[browser_tab_manager_visibility.js]
[browser_tab_move_to_new_window_reload.js]
[browser_tabbar_visibility.js]
[browser_tabCloseProbes.js]
[browser_tabContextMenu_keyboard.js]
[browser_tabReorder_overflow.js]
Expand Down
49 changes: 0 additions & 49 deletions browser/base/content/test/tabs/browser_tabbar_visibility.js

This file was deleted.

16 changes: 11 additions & 5 deletions browser/components/sessionstore/SessionStore.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ const WINDOW_HIDEABLE_FEATURES = [
"scrollbars",
];

const WINDOW_OPEN_FEATURES_MAP = {
locationbar: "location",
statusbar: "status",
};

// Messages that will be received via the Frame Message Manager.
const MESSAGES = [
// The content script sends us data that has been invalidated and needs to
Expand Down Expand Up @@ -5147,11 +5152,12 @@ var SessionStoreInternal = {
if (!hidden.length) {
features.push("all");
} else {
features.push(
...WINDOW_HIDEABLE_FEATURES.filter(aFeature => {
return !hidden.includes(aFeature);
})
);
features.push("resizable");
WINDOW_HIDEABLE_FEATURES.forEach(aFeature => {
if (!hidden.includes(aFeature)) {
features.push(WINDOW_OPEN_FEATURES_MAP[aFeature] || aFeature);
}
});
}
WINDOW_ATTRIBUTES.forEach(aFeature => {
// Use !isNaN as an easy way to ignore sizemode and check for numbers
Expand Down
1 change: 1 addition & 0 deletions browser/components/sessionstore/test/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ skip-if =
skip-if = debug || ccov # Bug 1625525
[browser_restore_srcdoc.js]
[browser_restore_tabless_window.js]
[browser_restored_window_features.js]
[browser_unrestored_crashedTabs.js]
skip-if =
!e10s || !crashreporter
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */

"use strict";

function testFeatures(win, barprops) {
for (let [name, visible] of Object.entries(barprops)) {
is(
win[name].visible,
visible,
name + " should be " + (visible ? "visible" : "hidden")
);
}
is(win.toolbar.visible, false, "toolbar should be hidden");
let toolbar = win.document.getElementById("TabsToolbar");
is(toolbar.collapsed, true, "tabbar should be collapsed");
let chromeFlags = win.docShell.treeOwner
.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIAppWindow).chromeFlags;
is(
chromeFlags & Ci.nsIWebBrowserChrome.CHROME_WINDOW_RESIZE,
Ci.nsIWebBrowserChrome.CHROME_WINDOW_RESIZE,
"window should be resizable"
);
}

add_task(async function testRestoredWindowFeatures() {
const DUMMY_PAGE = "browser/base/content/test/tabs/dummy_page.html";
const TESTS = [
{
url: "http://example.com/browser/" + DUMMY_PAGE,
features: "menubar=0,resizable",
barprops: { menubar: false },
},
{
url: "data:,", // title should be empty
features: "location,resizable",
barprops: { locationbar: true },
},
];
const TEST_URL_CHROME = "chrome://mochitests/content/browser/" + DUMMY_PAGE;

for (let test of TESTS) {
BrowserTestUtils.loadURI(gBrowser.selectedBrowser, TEST_URL_CHROME);
await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);

let newWindowPromise = BrowserTestUtils.waitForNewWindow({
url: test.url,
});
await SpecialPowers.spawn(gBrowser.selectedBrowser, [test], t => {
content.eval(`window.open("${t.url}", "_blank", "${t.features}")`);
});
let win = await newWindowPromise;
let title = win.document.title;

testFeatures(win, test.barprops);

await BrowserTestUtils.closeWindow(win);

newWindowPromise = BrowserTestUtils.waitForNewWindow({
url: test.url,
});
SessionStore.undoCloseWindow(0);
win = await newWindowPromise;

is(title, win.document.title, "title should be preserved");
testFeatures(win, test.barprops);

await BrowserTestUtils.closeWindow(win);
}
});

0 comments on commit 9aff9db

Please sign in to comment.