commit e12f6e82a50bc16fb40f51305532237dcd1c907f parent 1150c630e304aeb095ea7ee6b164a2dd34bbb8c4 Author: Sandor Molnar <smolnar@mozilla.com> Date: Thu, 20 Nov 2025 23:48:54 +0200 Revert "Bug 2000060 - add TabNotesController to connect tab notes to tabs r=dwalker,jswinarton,tabbrowser-reviewers" for causing bc failures This reverts commit 1150c630e304aeb095ea7ee6b164a2dd34bbb8c4. Diffstat:
13 files changed, 165 insertions(+), 403 deletions(-)
diff --git a/browser/components/tabbrowser/TabNotes.sys.mjs b/browser/components/tabbrowser/TabNotes.sys.mjs @@ -0,0 +1,79 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ + +export class TabNotesStorage { + /** @type {Map<URL, string>} */ + #store; + + constructor() { + this.reset(); + } + + /** + * Retrieve a note for a URL, if it exists. + * + * @param {URL} url + * The URL that the note is associated with + * @returns {string | undefined } + */ + get(url) { + return this.#store.get(url); + } + + /** + * Set a note for a URL. + * + * @param {URL} url + * The URL that the note should be associated with + * @param {string} note + * The note itself + * @returns {string} + * The actual note that was set after sanitization + */ + set(url, note) { + let sanitized = this.#sanitizeInput(note); + this.#store.set(url, sanitized); + return sanitized; + } + + /** + * Delete a note for a URL. + * + * @param {URL} url + * The URL of the note + * @returns {boolean} + * True if there was a note and it was deleted; false otherwise + */ + delete(url) { + return this.#store.delete(url); + } + + /** + * Check if a URL has a note. + * + * @param {URL} url + * The URL of the note + * @returns {boolean} + * True if a note is associated with this URL; false otherwise + */ + has(url) { + return this.#store.has(url); + } + + /** + * Clear all notes for all URLs. + * + * @returns {void} + */ + reset() { + this.#store = new Map(); + } + + #sanitizeInput(value) { + return value.slice(0, 1000); + } +} + +// Singleton object accessible from all windows +export const TabNotes = new TabNotesStorage(); diff --git a/browser/components/tabbrowser/content/tabbrowser.js b/browser/components/tabbrowser/content/tabbrowser.js @@ -112,6 +112,7 @@ "moz-src:///browser/components/newtab/SponsorProtection.sys.mjs", TabMetrics: "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs", + TabNotes: "moz-src:///browser/components/tabbrowser/TabNotes.sys.mjs", TabStateFlusher: "resource:///modules/sessionstore/TabStateFlusher.sys.mjs", TaskbarTabsUtils: @@ -9692,9 +9693,8 @@ var TabContextMenu = { let contextAddNote = document.getElementById("context_addNote"); let contextEditNote = document.getElementById("context_editNote"); if (gBrowser._tabNotesEnabled) { - let noteURL = this.contextTab.canonicalUrl; - let hasNote = this.TabNotes.has(noteURL); - contextAddNote.disabled = !noteURL; + let noteURL = this.contextTab.linkedBrowser.currentURI.spec; + let hasNote = gBrowser.TabNotes.has(noteURL); contextAddNote.hidden = hasNote; contextEditNote.hidden = !hasNote; } else { @@ -10252,5 +10252,4 @@ var TabContextMenu = { ChromeUtils.defineESModuleGetters(TabContextMenu, { GenAI: "resource:///modules/GenAI.sys.mjs", - TabNotes: "moz-src:///browser/components/tabnotes/TabNotes.sys.mjs", }); diff --git a/browser/components/tabbrowser/content/tabnote-menu.js b/browser/components/tabbrowser/content/tabnote-menu.js @@ -7,10 +7,6 @@ // This is loaded into chrome windows with the subscript loader. Wrap in // a block to prevent accidentally leaking globals onto `window`. { - const { TabNotes } = ChromeUtils.importESModule( - "moz-src:///browser/components/tabnotes/TabNotes.sys.mjs" - ); - class MozTabbrowserTabNoteMenu extends MozXULElement { static markup = /*html*/ ` <panel @@ -148,16 +144,12 @@ openPanel(tab) { this.#currentTab = tab; - let url = this.#currentTab.canonicalUrl; - - if (url) { - let note = TabNotes.get(url); - if (note) { - this.createMode = false; - this.#noteField.value = note; - } else { - this.createMode = true; - } + let url = this.#currentTab.linkedBrowser.currentURI.spec; + let note = gBrowser.TabNotes.get(url); + + if (note) { + this.createMode = false; + this.#noteField.value = note; } else { this.createMode = true; } @@ -176,9 +168,9 @@ } saveNote() { - let url = this.#currentTab.canonicalUrl; + let url = this.#currentTab.linkedBrowser.currentURI.spec; let note = this.#noteField.value; - TabNotes.set(url, note); + gBrowser.TabNotes.set(url, note); this.#panel.hidePopup(); } } diff --git a/browser/components/tabbrowser/moz.build b/browser/components/tabbrowser/moz.build @@ -14,6 +14,7 @@ MOZ_SRC_FILES += [ "OpenInTabsUtils.sys.mjs", "SmartTabGrouping.sys.mjs", "TabMetrics.sys.mjs", + "TabNotes.sys.mjs", "TabsList.sys.mjs", "TabUnloader.sys.mjs", ] diff --git a/browser/components/tabbrowser/test/browser/tabs/browser.toml b/browser/components/tabbrowser/test/browser/tabs/browser.toml @@ -581,9 +581,6 @@ tags = "vertical-tabs" tags = "vertical-tabs" ["browser_tab_notes.js"] -prefs = [ - "browser.tabs.notes.enabled=true", -] ["browser_tab_play.js"] tags = "vertical-tabs" diff --git a/browser/components/tabbrowser/test/browser/tabs/browser_tab_notes.js b/browser/components/tabbrowser/test/browser/tabs/browser_tab_notes.js @@ -4,12 +4,69 @@ "use strict"; -const { TabNotes } = ChromeUtils.importESModule( - "moz-src:///browser/components/tabnotes/TabNotes.sys.mjs" -); +add_setup(async function () { + await SpecialPowers.pushPrefEnv({ + set: [["browser.tabs.notes.enabled", true]], + }); -registerCleanupFunction(() => { - TabNotes.reset(); + registerCleanupFunction(async function () { + await SpecialPowers.popPrefEnv(); + }); +}); + +add_task(async function tabNotesBasicStorageTests() { + let url = "https://example.com/abc"; + let value = "some note"; + + let result = gBrowser.TabNotes.set(url, value); + Assert.equal(result, value, "TabNotes.set returns note value"); + + Assert.equal( + gBrowser.TabNotes.has(url), + true, + "TabNotes.has indicates that URL has a note" + ); + + Assert.equal( + gBrowser.TabNotes.get(url), + value, + "TabNotes.get returns previously set note value" + ); + + let fgWindow = await BrowserTestUtils.openNewBrowserWindow(); + Assert.equal( + fgWindow.gBrowser.TabNotes.get(url), + value, + "TabNotes.get returns previously set note value from any window" + ); + await BrowserTestUtils.closeWindow(fgWindow); + + gBrowser.TabNotes.delete(url); + + Assert.equal( + gBrowser.TabNotes.has(url), + false, + "TabNotes.has indicates that the deleted URL no longer has a note" + ); + + Assert.equal( + gBrowser.TabNotes.get(url), + undefined, + "TabNotes.get returns undefined for URL that does not have a note" + ); + + gBrowser.TabNotes.reset(); +}); + +add_task(async function tabNotesSanitizationTests() { + let url = "https://example.com/"; + let tooLongValue = "x".repeat(1500); + let correctValue = "x".repeat(1000); + + gBrowser.TabNotes.set(url, tooLongValue); + let result = gBrowser.TabNotes.get(url); + + Assert.equal(result, correctValue, "TabNotes.set truncates note length"); }); /** @@ -32,36 +89,6 @@ async function closeTabNoteMenu() { return menuHidden; } -/** - * Adds a tab, waits for it to load, and waits for the canonical URL to be - * identified from the loaded contents of the tab. This ensures that the tab - * meets the requirements to have a tab note. - * - * @param {string} [url="https://www.example.com"] - * @returns {Promise<{tab: MozTabbrowserTab, canonicalUrl: string}>} - */ -async function addTabWithCanonicalUrl(url = "https://www.example.com") { - const { promise, resolve } = Promise.withResolvers(); - BrowserTestUtils.addTab( - gBrowser, - url, - { - skipAnimation: true, - }, - tab => { - BrowserTestUtils.waitForEvent( - window, - "CanonicalURL:Identified", - false, - e => e.target == tab.linkedBrowser - ).then(e => { - resolve({ tab, canonicalUrl: e.detail.canonicalUrl }); - }); - } - ); - return promise; -} - add_task(async function test_openTabNotePanelFromContextMenu() { // open context menu with tab notes disabled await SpecialPowers.pushPrefEnv({ @@ -104,6 +131,10 @@ add_task(async function test_openTabNotePanelFromContextMenu() { }); add_task(async function test_dismissTabNotePanel() { + await SpecialPowers.pushPrefEnv({ + set: [["browser.tabs.notes.enabled", true]], + }); + // Dismiss panel by pressing Esc let tab = await addTab("about:blank"); let tabNoteMenu = await openTabNoteMenu(tab); @@ -129,18 +160,23 @@ add_task(async function test_dismissTabNotePanel() { "Tab note menu closes after clicking cancel button" ); BrowserTestUtils.removeTab(tab); + await SpecialPowers.popPrefEnv(); }); add_task(async function test_saveTabNote() { - let { tab, canonicalUrl } = await addTabWithCanonicalUrl(); + await SpecialPowers.pushPrefEnv({ + set: [["browser.tabs.notes.enabled", true]], + }); + let tab = await addTab("about:blank"); let tabNoteMenu = await openTabNoteMenu(tab); tabNoteMenu.querySelector("textarea").value = "Lorem ipsum dolor"; let menuHidden = BrowserTestUtils.waitForPopupEvent(tabNoteMenu, "hidden"); tabNoteMenu.querySelector("#tab-note-editor-button-save").click(); await menuHidden; - Assert.equal(TabNotes.get(canonicalUrl), "Lorem ipsum dolor"); + Assert.equal(gBrowser.TabNotes.get("about:blank"), "Lorem ipsum dolor"); - TabNotes.delete(canonicalUrl); + gBrowser.TabNotes.delete("about:blank"); BrowserTestUtils.removeTab(tab); + await SpecialPowers.popPrefEnv(); }); diff --git a/browser/components/tabnotes/TabNotes.sys.mjs b/browser/components/tabnotes/TabNotes.sys.mjs @@ -1,93 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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/. */ - -/** - * Provides the CRUD interface for tab notes. - */ -export class TabNotesStorage { - /** @type {Map<URL, string>} */ - #store; - - constructor() { - this.reset(); - } - - /** - * Retrieve a note for a URL, if it exists. - * - * @param {URL} url - * The URL that the note is associated with - * @returns {string | undefined } - */ - get(url) { - return this.#store.get(url); - } - - /** - * Set a note for a URL. - * - * @param {URL} url - * The URL that the note should be associated with - * @param {string} note - * The note itself - * @returns {string} - * The actual note that was set after sanitization - */ - set(url, note) { - let existingNote = this.get(url); - let sanitized = this.#sanitizeInput(note); - this.#store.set(url, sanitized); - if (!existingNote) { - Services.obs.notifyObservers(null, "TabNote:Created", url.toString()); - } else if (existingNote && existingNote != sanitized) { - Services.obs.notifyObservers(null, "TabNote:Edited", url.toString()); - } - - return sanitized; - } - - /** - * Delete a note for a URL. - * - * @param {URL} url - * The URL of the note - * @returns {boolean} - * True if there was a note and it was deleted; false otherwise - */ - delete(url) { - let wasDeleted = this.#store.delete(url); - if (wasDeleted) { - Services.obs.notifyObservers(null, "TabNote:Removed", url.toString()); - } - return wasDeleted; - } - - /** - * Check if a URL has a note. - * - * @param {URL} url - * The URL of the note - * @returns {boolean} - * True if a note is associated with this URL; false otherwise - */ - has(url) { - return this.#store.has(url); - } - - /** - * Clear all notes for all URLs. - * - * @returns {void} - */ - reset() { - this.#store = new Map(); - } - - #sanitizeInput(value) { - return value.slice(0, 1000); - } -} - -// Singleton object accessible from all windows -export const TabNotes = new TabNotesStorage(); diff --git a/browser/components/tabnotes/TabNotesController.sys.mjs b/browser/components/tabnotes/TabNotesController.sys.mjs @@ -1,165 +0,0 @@ -/** - * This Source Code Form is subject to the terms of the Mozilla Public - * 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/. - */ -import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; - -const lazy = {}; -ChromeUtils.defineESModuleGetters(lazy, { - BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.sys.mjs", - TabNotes: "moz-src:///browser/components/tabnotes/TabNotes.sys.mjs", -}); -ChromeUtils.defineLazyGetter(lazy, "logConsole", function () { - return console.createInstance({ - prefix: "TabNotes", - maxLogLevel: Services.prefs.getBoolPref("browser.tabs.notes.debug", false) - ? "Debug" - : "Warn", - }); -}); -XPCOMUtils.defineLazyPreferenceGetter( - lazy, - "TAB_NOTES_ENABLED", - "browser.tabs.notes.enabled", - false -); - -const EVENTS = ["CanonicalURL:Identified"]; - -const TOPICS = ["TabNote:Created", "TabNote:Edited", "TabNote:Removed"]; - -/** - * Orchestrates the tab notes life cycle. - * - * Singleton class within Firefox that observes tab notes-related topics, - * listens for tab notes-related events, and ensures that the state of the - * tabbrowser stays in sync with the TabNotes repository. - * - * Registers with the category manager in order to initialize on Firefox - * startup and be notified when windows are opened/closed. - * - * @see https://firefox-source-docs.mozilla.org/browser/CategoryManagerIndirection.html - */ -class TabNotesControllerClass { - /** - * Registered with `browser-first-window-ready` to be notified of - * app startup. - * - * @see tabnotes.manifest - */ - init() { - if (lazy.TAB_NOTES_ENABLED) { - TOPICS.forEach(topicName => Services.obs.addObserver(this, topicName)); - lazy.logConsole.debug("init", TOPICS); - } else { - lazy.logConsole.info("Tab notes disabled"); - } - } - - /** - * Registered with `browser-window-delayed-startup` to be notified of new - * windows. - * - * @param {Window} win - * @see tabnotes.manifest - */ - registerWindow(win) { - if (lazy.TAB_NOTES_ENABLED) { - EVENTS.forEach(eventName => win.addEventListener(eventName, this)); - lazy.logConsole.debug("registerWindow", EVENTS, win); - } - } - - /** - * Registered with `browser-window-unload` to be notified of unloaded windows. - * - * @param {Window} win - * @see tabnotes.manifest - */ - unregisterWindow(win) { - if (lazy.TAB_NOTES_ENABLED) { - EVENTS.forEach(eventName => win.removeEventListener(eventName, this)); - lazy.logConsole.debug("unregisterWindow", EVENTS, win); - } - } - - /** - * Registered with `browser-quit-application-granted` to be notified of - * app shutdown. - * - * @see tabnotes.manifest - */ - quit() { - if (lazy.TAB_NOTES_ENABLED) { - TOPICS.forEach(topicName => Services.obs.removeObserver(this, topicName)); - lazy.logConsole.debug("quit", TOPICS); - } - } - - /** - * @param {CanonicalURLIdentifiedEvent} event - */ - handleEvent(event) { - switch (event.type) { - case "CanonicalURL:Identified": - { - // A browser identified its canonical URL, so we can determine whether - // the tab has an associated note and should therefore display a tab - // notes icon. - const browser = event.target; - const { canonicalUrl } = event.detail; - const gBrowser = browser.getTabBrowser(); - const tab = gBrowser.getTabForBrowser(browser); - tab.canonicalUrl = canonicalUrl; - tab.hasTabNote = lazy.TabNotes.has(canonicalUrl); - lazy.logConsole.debug("CanonicalURL:Identified", tab, canonicalUrl); - } - break; - } - } - - /** - * @param {nsISupports} subject - * @param {string} topic - * @param {string} data - */ - observe(subject, topic, data) { - switch (topic) { - case "TabNote:Created": - { - // A new tab note was created for a specific canonical URL. Ensure that - // all tabs with the same canonical URL also indicate that there is a - // tab note. - const canonicalUrl = data; - for (const win of lazy.BrowserWindowTracker.orderedWindows) { - for (const tab of win.gBrowser.tabs) { - if (tab.canonicalUrl == canonicalUrl) { - tab.hasTabNote = true; - } - } - } - lazy.logConsole.debug("TabNote:Created", canonicalUrl); - } - break; - case "TabNote:Removed": - { - // A new tab note was removed from a specific canonical URL. Ensure that - // all tabs with the same canonical URL also indicate that there is no - // longer a tab note. - const canonicalUrl = data; - for (const win of lazy.BrowserWindowTracker.orderedWindows) { - for (const tab of win.gBrowser.tabs) { - if (tab.canonicalUrl == canonicalUrl) { - tab.hasTabNote = false; - } - } - } - lazy.logConsole.debug("TabNote:Removed", canonicalUrl); - } - break; - } - } -} - -export const TabNotesController = new TabNotesControllerClass(); diff --git a/browser/components/tabnotes/moz.build b/browser/components/tabnotes/moz.build @@ -9,19 +9,9 @@ with Files("**"): MOZ_SRC_FILES += [ "CanonicalURL.sys.mjs", - "TabNotes.sys.mjs", - "TabNotesController.sys.mjs", ] FINAL_TARGET_FILES.actors += [ "CanonicalURLChild.sys.mjs", "CanonicalURLParent.sys.mjs", ] - -EXTRA_COMPONENTS += [ - "tabnotes.manifest", -] - -XPCSHELL_TESTS_MANIFESTS += [ - "test/unit/xpcshell.toml", -] diff --git a/browser/components/tabnotes/tabnotes.manifest b/browser/components/tabnotes/tabnotes.manifest @@ -1,4 +0,0 @@ -category browser-first-window-ready moz-src:///browser/components/tabnotes/TabNotesController.sys.mjs TabNotesController.init -category browser-window-delayed-startup moz-src:///browser/components/tabnotes/TabNotesController.sys.mjs TabNotesController.registerWindow -category browser-window-unload moz-src:///browser/components/tabnotes/TabNotesController.sys.mjs TabNotesController.unregisterWindow -category browser-quit-application-granted moz-src:///browser/components/tabnotes/TabNotesController.sys.mjs TabNotesController.quit diff --git a/browser/components/tabnotes/test/unit/test_tab_notes.js b/browser/components/tabnotes/test/unit/test_tab_notes.js @@ -1,56 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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/. */ - -const { TabNotes } = ChromeUtils.importESModule( - "moz-src:///browser/components/tabnotes/TabNotes.sys.mjs" -); - -add_task(async function tabNotesBasicStorageTests() { - let url = "https://example.com/abc"; - let value = "some note"; - - let result = TabNotes.set(url, value); - Assert.equal(result, value, "TabNotes.set returns note value"); - - Assert.equal( - TabNotes.has(url), - true, - "TabNotes.has indicates that URL has a note" - ); - - Assert.equal( - TabNotes.get(url), - value, - "TabNotes.get returns previously set note value" - ); - - TabNotes.delete(url); - - Assert.equal( - TabNotes.has(url), - false, - "TabNotes.has indicates that the deleted URL no longer has a note" - ); - - Assert.equal( - TabNotes.get(url), - undefined, - "TabNotes.get returns undefined for URL that does not have a note" - ); - - TabNotes.reset(); -}); - -add_task(async function tabNotesSanitizationTests() { - let url = "https://example.com/"; - let tooLongValue = "x".repeat(1500); - let correctValue = "x".repeat(1000); - - TabNotes.set(url, tooLongValue); - let result = TabNotes.get(url); - - Assert.equal(result, correctValue, "TabNotes.set truncates note length"); - - TabNotes.reset(); -}); diff --git a/browser/components/tabnotes/test/unit/xpcshell.toml b/browser/components/tabnotes/test/unit/xpcshell.toml @@ -1,6 +0,0 @@ -[DEFAULT] -prefs = [ - "browser.tabs.notes.enabled=true", -] - -["test_tab_notes.js"] diff --git a/browser/components/tabnotes/types/tabnotes.ts b/browser/components/tabnotes/types/tabnotes.ts @@ -6,11 +6,3 @@ type CanonicalURLSource = "link" | "opengraph" | "jsonLd" | "fallback"; type CanonicalURLSourceResults = { [source in CanonicalURLSource]: string | null; }; - -interface CanonicalURLIdentifiedEvent { - type: "CanonicalURL:Identified"; - target: MozBrowser; - detail: { - canonicalUrl: string; - }; -}