commit 1150c630e304aeb095ea7ee6b164a2dd34bbb8c4 parent 0a437e60ab98a0ce22ace3522192cf31288818cf Author: Stephen Thompson <sthompson@mozilla.com> Date: Thu, 20 Nov 2025 21:14:43 +0000 Bug 2000060 - add TabNotesController to connect tab notes to tabs r=dwalker,jswinarton,tabbrowser-reviewers TabNotesController listens to window events and observes Tab Notes topics to ensure that the Tabbrowser state is in sync with what's happening with tab notes. - TabNotesController sets a `canonicalUrl` property on each tab that is navigated to an HTTP(S) web page with an identified canonical URL - TabNotesController ensures that all tabs with the same canonical URL update their tab note status in response to any change in a tab note I updated the TabNotes singleton so that it also notifies observers about tab notes events. I also moved the TabNotes singleton out of the tabbrowser component and into the tabnotes component so that Tabbrowser doesn't need to know as much about tab notes. Since TabNotes is no longer attached to a browser window's gBrowser, I converted the existing browser_tab_notes browser test into an XPCShell unit test. With these changes, it should be possible to do mostly normal web browsing and expect to see the tab note icon appear and disappear on the appropriate tab(s) based on any operations made with TabNotes. There are still a number of gaps like the browser back/forward buttons, single-page apps, navigating to non-HTTP(S) pages like about: pages, etc. where the tabs don't correctly reflect the state of the tab notes. Differential Revision: https://phabricator.services.mozilla.com/D273103 Diffstat:
13 files changed, 403 insertions(+), 165 deletions(-)
diff --git a/browser/components/tabbrowser/TabNotes.sys.mjs b/browser/components/tabbrowser/TabNotes.sys.mjs @@ -1,79 +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/. */ - -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,7 +112,6 @@ "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: @@ -9693,8 +9692,9 @@ var TabContextMenu = { let contextAddNote = document.getElementById("context_addNote"); let contextEditNote = document.getElementById("context_editNote"); if (gBrowser._tabNotesEnabled) { - let noteURL = this.contextTab.linkedBrowser.currentURI.spec; - let hasNote = gBrowser.TabNotes.has(noteURL); + let noteURL = this.contextTab.canonicalUrl; + let hasNote = this.TabNotes.has(noteURL); + contextAddNote.disabled = !noteURL; contextAddNote.hidden = hasNote; contextEditNote.hidden = !hasNote; } else { @@ -10252,4 +10252,5 @@ 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,6 +7,10 @@ // 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 @@ -144,12 +148,16 @@ openPanel(tab) { this.#currentTab = tab; - let url = this.#currentTab.linkedBrowser.currentURI.spec; - let note = gBrowser.TabNotes.get(url); - - if (note) { - this.createMode = false; - this.#noteField.value = note; + 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; + } } else { this.createMode = true; } @@ -168,9 +176,9 @@ } saveNote() { - let url = this.#currentTab.linkedBrowser.currentURI.spec; + let url = this.#currentTab.canonicalUrl; let note = this.#noteField.value; - gBrowser.TabNotes.set(url, note); + TabNotes.set(url, note); this.#panel.hidePopup(); } } diff --git a/browser/components/tabbrowser/moz.build b/browser/components/tabbrowser/moz.build @@ -14,7 +14,6 @@ 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,6 +581,9 @@ 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,69 +4,12 @@ "use strict"; -add_setup(async function () { - await SpecialPowers.pushPrefEnv({ - set: [["browser.tabs.notes.enabled", true]], - }); +const { TabNotes } = ChromeUtils.importESModule( + "moz-src:///browser/components/tabnotes/TabNotes.sys.mjs" +); - 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"); +registerCleanupFunction(() => { + TabNotes.reset(); }); /** @@ -89,6 +32,36 @@ 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({ @@ -131,10 +104,6 @@ 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); @@ -160,23 +129,18 @@ 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() { - await SpecialPowers.pushPrefEnv({ - set: [["browser.tabs.notes.enabled", true]], - }); - let tab = await addTab("about:blank"); + let { tab, canonicalUrl } = await addTabWithCanonicalUrl(); 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(gBrowser.TabNotes.get("about:blank"), "Lorem ipsum dolor"); + Assert.equal(TabNotes.get(canonicalUrl), "Lorem ipsum dolor"); - gBrowser.TabNotes.delete("about:blank"); + TabNotes.delete(canonicalUrl); BrowserTestUtils.removeTab(tab); - await SpecialPowers.popPrefEnv(); }); diff --git a/browser/components/tabnotes/TabNotes.sys.mjs b/browser/components/tabnotes/TabNotes.sys.mjs @@ -0,0 +1,93 @@ +/* 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 @@ -0,0 +1,165 @@ +/** + * 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,9 +9,19 @@ 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 @@ -0,0 +1,4 @@ +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 @@ -0,0 +1,56 @@ +/* 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 @@ -0,0 +1,6 @@ +[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,3 +6,11 @@ type CanonicalURLSource = "link" | "opengraph" | "jsonLd" | "fallback"; type CanonicalURLSourceResults = { [source in CanonicalURLSource]: string | null; }; + +interface CanonicalURLIdentifiedEvent { + type: "CanonicalURL:Identified"; + target: MozBrowser; + detail: { + canonicalUrl: string; + }; +}