From f929300a98e752833d0c1aa36612d81f1f06a173 Mon Sep 17 00:00:00 2001
From: Nika Layzell <nika@thelayzells.com>
Date: Thu, 25 Apr 2024 18:04:25 +0000
Subject: [PATCH] Bug 1887029 - Simplify TabAttributes to explicitly specify
supported attributes, a=dmeehan
The only tab attribute which is ever persisted by SessionStore is
"customizemode". This patch limits the logic to only allow persisting and
restoring this attribute.
The browser_attributes.js test is also updated to use the "customizemode"
attribute for testing, rather than a custom specified attribute.
Minor test fix for ESR115.
Differential Revision: https://phabricator.services.mozilla.com/D208655
---
.../customizableui/CustomizeMode.sys.mjs | 2 -
.../sessionstore/SessionStore.sys.mjs | 17 ------
.../sessionstore/TabAttributes.sys.mjs | 43 +++-----------
.../sessionstore/test/browser_attributes.js | 59 +++++++++++++------
4 files changed, 48 insertions(+), 73 deletions(-)
diff --git a/browser/components/customizableui/CustomizeMode.sys.mjs b/browser/components/customizableui/CustomizeMode.sys.mjs
index 86c5e1b786b1e..72888a7248e70 100644
--- a/browser/components/customizableui/CustomizeMode.sys.mjs
+++ b/browser/components/customizableui/CustomizeMode.sys.mjs
@@ -27,7 +27,6 @@ const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
AddonManager: "resource://gre/modules/AddonManager.sys.mjs",
DragPositionManager: "resource:///modules/DragPositionManager.sys.mjs",
- SessionStore: "resource:///modules/sessionstore/SessionStore.sys.mjs",
URILoadingHelper: "resource:///modules/URILoadingHelper.sys.mjs",
});
ChromeUtils.defineModuleGetter(
@@ -225,7 +224,6 @@ CustomizeMode.prototype = {
gTab = aTab;
gTab.setAttribute("customizemode", "true");
- lazy.SessionStore.persistTabAttribute("customizemode");
if (gTab.linkedPanel) {
gTab.linkedBrowser.stop();
diff --git a/browser/components/sessionstore/SessionStore.sys.mjs b/browser/components/sessionstore/SessionStore.sys.mjs
index 2c50de65f8368..82e5b67481755 100644
--- a/browser/components/sessionstore/SessionStore.sys.mjs
+++ b/browser/components/sessionstore/SessionStore.sys.mjs
@@ -434,10 +434,6 @@ export var SessionStore = {
SessionStoreInternal.deleteCustomGlobalValue(aKey);
},
- persistTabAttribute: function ss_persistTabAttribute(aName) {
- SessionStoreInternal.persistTabAttribute(aName);
- },
-
restoreLastSession: function ss_restoreLastSession() {
SessionStoreInternal.restoreLastSession();
},
@@ -3615,12 +3611,6 @@ var SessionStoreInternal = {
this.saveStateDelayed();
},
- persistTabAttribute: function ssi_persistTabAttribute(aName) {
- if (lazy.TabAttributes.persist(aName)) {
- this.saveStateDelayed();
- }
- },
-
/**
* Undoes the closing of a tab or window which corresponds
* to the closedId passed in.
@@ -4782,13 +4772,6 @@ var SessionStoreInternal = {
tab.updateLastAccessed(tabData.lastAccessed);
}
- if ("attributes" in tabData) {
- // Ensure that we persist tab attributes restored from previous sessions.
- Object.keys(tabData.attributes).forEach(a =>
- lazy.TabAttributes.persist(a)
- );
- }
-
if (!tabData.entries) {
tabData.entries = [];
}
diff --git a/browser/components/sessionstore/TabAttributes.sys.mjs b/browser/components/sessionstore/TabAttributes.sys.mjs
index 1c7f54b6abcbe..ea53156d129cb 100644
--- a/browser/components/sessionstore/TabAttributes.sys.mjs
+++ b/browser/components/sessionstore/TabAttributes.sys.mjs
@@ -2,27 +2,13 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
-// We never want to directly read or write these attributes.
-// 'image' should not be accessed directly but handled by using the
-// gBrowser.getIcon()/setIcon() methods.
-// 'muted' should not be accessed directly but handled by using the
-// tab.linkedBrowser.audioMuted/toggleMuteAudio methods.
-// 'pending' is used internal by sessionstore and managed accordingly.
-const ATTRIBUTES_TO_SKIP = new Set([
- "image",
- "muted",
- "pending",
- "skipbackgroundnotify",
-]);
+// Tab attributes which are persisted & restored by SessionStore.
+const PERSISTED_ATTRIBUTES = ["customizemode"];
// A set of tab attributes to persist. We will read a given list of tab
// attributes when collecting tab data and will re-set those attributes when
// the given tab data is restored to a new tab.
export var TabAttributes = Object.freeze({
- persist(name) {
- return TabAttributesInternal.persist(name);
- },
-
get(tab) {
return TabAttributesInternal.get(tab);
},
@@ -33,21 +19,10 @@ export var TabAttributes = Object.freeze({
});
var TabAttributesInternal = {
- _attrs: new Set(),
-
- persist(name) {
- if (this._attrs.has(name) || ATTRIBUTES_TO_SKIP.has(name)) {
- return false;
- }
-
- this._attrs.add(name);
- return true;
- },
-
get(tab) {
let data = {};
- for (let name of this._attrs) {
+ for (let name of PERSISTED_ATTRIBUTES) {
if (tab.hasAttribute(name)) {
data[name] = tab.getAttribute(name);
}
@@ -57,15 +32,11 @@ var TabAttributesInternal = {
},
set(tab, data = {}) {
- // Clear attributes.
- for (let name of this._attrs) {
+ // Clear & Set attributes.
+ for (let name of PERSISTED_ATTRIBUTES) {
tab.removeAttribute(name);
- }
-
- // Set attributes.
- for (let [name, value] of Object.entries(data)) {
- if (!ATTRIBUTES_TO_SKIP.has(name)) {
- tab.setAttribute(name, value);
+ if (name in data) {
+ tab.setAttribute(name, data[name]);
}
}
},
diff --git a/browser/components/sessionstore/test/browser_attributes.js b/browser/components/sessionstore/test/browser_attributes.js
index 336573a50862a..db349a31a2514 100644
--- a/browser/components/sessionstore/test/browser_attributes.js
+++ b/browser/components/sessionstore/test/browser_attributes.js
@@ -38,45 +38,68 @@ add_task(async function test() {
ok(tab.hasAttribute("muted"), "tab.muted exists");
// Make sure we do not persist 'image' and 'muted' attributes.
- ss.persistTabAttribute("image");
- ss.persistTabAttribute("muted");
let { attributes } = JSON.parse(ss.getTabState(tab));
ok(!("image" in attributes), "'image' attribute not saved");
ok(!("muted" in attributes), "'muted' attribute not saved");
- ok(!("custom" in attributes), "'custom' attribute not saved");
-
- // Test persisting a custom attribute.
- tab.setAttribute("custom", "foobar");
- ss.persistTabAttribute("custom");
-
- ({ attributes } = JSON.parse(ss.getTabState(tab)));
- is(attributes.custom, "foobar", "'custom' attribute is correct");
-
- // Make sure we're backwards compatible and restore old 'image' attributes.
+ ok(!("customizemode" in attributes), "'customizemode' attribute not saved");
+
+ // Test persisting a customizemode attribute.
+ {
+ let customizationReady = BrowserTestUtils.waitForEvent(
+ gNavToolbox,
+ "customizationready"
+ );
+ gCustomizeMode.enter();
+ await customizationReady;
+ }
+
+ let customizeIcon = gBrowser.getIcon(gBrowser.selectedTab);
+ ({ attributes } = JSON.parse(ss.getTabState(gBrowser.selectedTab)));
+ ok(!("image" in attributes), "'image' attribute not saved");
+ is(attributes.customizemode, "true", "'customizemode' attribute is correct");
+
+ {
+ let afterCustomization = BrowserTestUtils.waitForEvent(
+ gNavToolbox,
+ "aftercustomization"
+ );
+ gCustomizeMode.exit();
+ await afterCustomization;
+ }
+
+ // Test restoring a customizemode tab.
let state = {
- entries: [{ url: "about:mozilla", triggeringPrincipal_base64 }],
- attributes: { custom: "foobaz" },
- image: gBrowser.getIcon(tab),
+ entries: [],
+ attributes: { customizemode: "true", nonpersisted: "true" },
};
+ // Customize mode doesn't like being restored on top of a non-blank tab.
+ // For the moment, it appears it isn't possible to restore customizemode onto
+ // an existing non-blank tab outside of tests, however this may be a latent
+ // bug if we ever try to do that in the future.
+ let principal = Services.scriptSecurityManager.createNullPrincipal({});
+ tab.linkedBrowser.createAboutBlankContentViewer(principal, principal);
+
// Prepare a pending tab waiting to be restored.
let promise = promiseTabRestoring(tab);
ss.setTabState(tab, JSON.stringify(state));
await promise;
ok(tab.hasAttribute("pending"), "tab is pending");
- is(gBrowser.getIcon(tab), state.image, "tab has correct icon");
+ ok(tab.hasAttribute("customizemode"), "tab is in customizemode");
+ ok(!tab.hasAttribute("nonpersisted"), "tab has no nonpersisted attribute");
+ is(gBrowser.getIcon(tab), customizeIcon, "tab has correct icon");
ok(!state.attributes.image, "'image' attribute not saved");
// Let the pending tab load.
gBrowser.selectedTab = tab;
- await promiseTabRestored(tab);
// Ensure no 'image' or 'pending' attributes are stored.
({ attributes } = JSON.parse(ss.getTabState(tab)));
ok(!("image" in attributes), "'image' attribute not saved");
ok(!("pending" in attributes), "'pending' attribute not saved");
- is(attributes.custom, "foobaz", "'custom' attribute is correct");
+ ok(!("nonpersisted" in attributes), "'nonpersisted' attribute not saved");
+ is(attributes.customizemode, "true", "'customizemode' attribute is correct");
// Clean up.
gBrowser.removeTab(tab);
--
GitLab