From 9a440e075db4499705218d66ff2bfbce34f5b6bc Mon Sep 17 00:00:00 2001 From: Jared Hirsch Date: Wed, 13 Nov 2024 02:46:30 +0000 Subject: [PATCH] Bug 1926145 - Add profiles menu to menubar. r=niklas,fluent-reviewers,desktop-theme-reviewers,dao,frontend-codestyle-reviewers Differential Revision: https://phabricator.services.mozilla.com/D226412 --- .eslintrc-test-paths.js | 1 + .../base/content/appmenu-viewcache.inc.xhtml | 2 - browser/base/content/browser-menubar.inc | 5 + browser/base/content/browser-menubar.js | 3 + browser/base/content/browser-profiles.js | 210 ++++++++++++++---- browser/base/content/browser-sets.inc | 3 + browser/base/content/browser-sets.js | 6 + .../profiles/tests/browser/browser.toml | 6 + .../tests/browser/browser_menubar_profiles.js | 64 ++++++ browser/locales/en-US/browser/menubar.ftl | 9 + browser/themes/shared/browser-shared.css | 8 + .../shared/customizableui/panelUI-shared.css | 13 +- 12 files changed, 281 insertions(+), 49 deletions(-) create mode 100644 browser/components/profiles/tests/browser/browser_menubar_profiles.js diff --git a/.eslintrc-test-paths.js b/.eslintrc-test-paths.js index 17c814cd2f44..9cc1d9684d4e 100644 --- a/.eslintrc-test-paths.js +++ b/.eslintrc-test-paths.js @@ -105,6 +105,7 @@ const extraBrowserTestPaths = [ "browser/base/content/test/popupNotifications/", "browser/base/content/test/popups/", "browser/base/content/test/privateBrowsing/", + "browser/base/content/test/profiles/", "browser/base/content/test/protectionsUI/", "browser/base/content/test/referrer/", "browser/base/content/test/sanitize/", diff --git a/browser/base/content/appmenu-viewcache.inc.xhtml b/browser/base/content/appmenu-viewcache.inc.xhtml index a2fb101fffe7..3eb65bddbe74 100644 --- a/browser/base/content/appmenu-viewcache.inc.xhtml +++ b/browser/base/content/appmenu-viewcache.inc.xhtml @@ -201,7 +201,6 @@ /> -#ifdef MOZ_SELECTABLE_PROFILES @@ -239,7 +238,6 @@ /> -#endif diff --git a/browser/base/content/browser-menubar.inc b/browser/base/content/browser-menubar.inc index 574cfa1004d8..2e5478c307db 100644 --- a/browser/base/content/browser-menubar.inc +++ b/browser/base/content/browser-menubar.inc @@ -328,6 +328,11 @@ + + SelectableProfileService?.isEnabled ); - if (!this.PROFILES_ENABLED) { - return; - } + await this.toggleProfileMenus(); + }, + + async toggleProfileMenus() { + let profilesMenu = document.getElementById("profiles-menu"); + profilesMenu.hidden = !this.PROFILES_ENABLED; await this.toggleProfileButtonVisibility(); }, + /** + * Draws the app menu toolbarbutton. + */ async toggleProfileButtonVisibility() { let profilesButton = PanelMultiView.getViewNode( document, @@ -44,12 +58,12 @@ var gProfiles = { if (!this.PROFILES_ENABLED) { document.l10n.setAttributes(profilesButton, "appmenu-profiles"); profilesButton.classList.remove("subviewbutton-iconic"); - profilesButton.removeEventListener("command", this); - subview.removeEventListener("command", this); + profilesButton.removeEventListener("command", this.handleCommand); + subview.removeEventListener("command", this.handleCommand); return; } - profilesButton.addEventListener("command", this); - subview.addEventListener("command", this); + profilesButton.addEventListener("command", this.handleCommand); + subview.addEventListener("command", this.handleCommand); // If the feature is preffed on, but we haven't created profiles yet, the // service will not be initialized. @@ -63,7 +77,8 @@ var gProfiles = { } let { themeBg, themeFg } = SelectableProfileService.currentProfile.theme; - profilesButton.style.cssText = `--themeBg: ${themeBg}; --themeFg: ${themeFg};`; + profilesButton.style.setProperty("--appmenu-profiles-theme-bg", themeBg); + profilesButton.style.setProperty("--appmenu-profiles-theme-fg", themeFg); profilesButton.classList.add("subviewbutton-iconic"); profilesButton.setAttribute( @@ -77,13 +92,95 @@ var gProfiles = { ); }, - updateView(panel) { - this.populateSubView(); + /** + * Draws the menubar panel contents. + */ + onPopupShowing() { + // TODO (bug 1926630) We cannot async fetch the current list of profiles + // because menubar popups do not support async popupshowing callbacks + // (the resulting menu is not rendered correctly on macos). + // + // Our temporary workaround is to use a stale cached copy of the profiles + // list to render synchronously, and update our profiles list async. If the + // profiles datastore has been updated since the popup was last shown, the + // contents of the menu will be stale on the first render, then up-to-date + // after that. + // + // Bug 1926630 will ensure correct menu contents by updating + // `this.profiles` in response to a notification from the + // SelectableProfileService, and we can remove this call then. + SelectableProfileService.getAllProfiles().then(profiles => { + this.profiles = profiles; + }); + + let menuPopup = document.getElementById("menu_ProfilesPopup"); + + while (menuPopup.hasChildNodes()) { + menuPopup.firstChild.remove(); + } + + let profiles = this.profiles; + let currentProfile = SelectableProfileService.currentProfile; + + for (let profile of profiles) { + let menuitem = document.createXULElement("menuitem"); + let { themeBg, themeFg } = profile.theme; + menuitem.setAttribute("profileid", profile.id); + menuitem.setAttribute("command", "Profiles:LaunchProfile"); + menuitem.setAttribute("label", profile.name); + menuitem.style.setProperty("--menu-profiles-theme-bg", themeBg); + menuitem.style.setProperty("--menu-profiles-theme-fg", themeFg); + menuitem.style.listStyleImage = `url(chrome://browser/content/profiles/assets/48_${profile.avatar}.svg)`; + menuitem.classList.add("menuitem-iconic", "menuitem-iconic-profile"); + + if (profile.id === currentProfile.id) { + menuitem.classList.add("current"); + menuitem.setAttribute("type", "checkbox"); + menuitem.setAttribute("checked", "true"); + } + + menuPopup.appendChild(menuitem); + } + + let newProfile = document.createXULElement("menuitem"); + newProfile.id = "menu_newProfile"; + newProfile.setAttribute("command", "Profiles:CreateProfile"); + newProfile.setAttribute("data-l10n-id", "menu-profiles-new-profile"); + menuPopup.appendChild(newProfile); + + let separator = document.createXULElement("menuseparator"); + separator.id = "profilesSeparator"; + menuPopup.appendChild(separator); + + let manageProfiles = document.createXULElement("menuitem"); + manageProfiles.id = "menu_manageProfiles"; + manageProfiles.setAttribute("command", "Profiles:ManageProfiles"); + manageProfiles.setAttribute( + "data-l10n-id", + "menu-profiles-manage-profiles" + ); + menuPopup.appendChild(manageProfiles); + }, + + manageProfiles() { + return SelectableProfileService.maybeSetupDataStore().then(() => { + window.openDialog( + "about:profilemanager", + "_blank", + "centerscreen,chrome,titlebar" + ); + }); + }, + + createNewProfile() { + SelectableProfileService.createNewProfile(); + }, + + async updateView(panel) { + await this.populateSubView(); PanelUI.showSubView("PanelUI-profiles", panel); }, - // Note: Not async because the browser-sets.js handler is not async. - // This will be an issue when we add menubar menuitems. launchProfile(aEvent) { SelectableProfileService.getProfile( aEvent.target.getAttribute("profileid") @@ -92,32 +189,54 @@ var gProfiles = { }); }, - async handleEvent(aEvent) { - let id = aEvent.target.id; - switch (aEvent.type) { - case "command": { - if (id == "appMenu-profiles-button") { - this.updateView(aEvent.target); - } else if (id == "profiles-appmenu-back-button") { - aEvent.target.closest("panelview").panelMultiView.goBack(); - aEvent.target.blur(); - } else if (id == "profiles-edit-this-profile-button") { - openTrustedLinkIn("about:editprofile", "tab"); - } else if (id == "profiles-manage-profiles-button") { - // TODO: (Bug 1924827) Open in a dialog, not a tab. - openTrustedLinkIn("about:profilemanager", "tab"); - } else if (id == "profiles-create-profile-button") { - SelectableProfileService.createNewProfile(); - } else if (aEvent.target.classList.contains("profile-item")) { - // moved to a helper to expose to the menubar commands - this.launchProfile(aEvent); - } + handleCommand(aEvent) { + switch (aEvent.target.id) { + /* Appmenu events */ + case "appMenu-profiles-button": { + this.updateView(aEvent.target); + break; + } + case "profiles-appmenu-back-button": { + aEvent.target.closest("panelview").panelMultiView.goBack(); + aEvent.target.blur(); + break; + } + case "profiles-edit-this-profile-button": { + openTrustedLinkIn("about:editprofile", "tab"); + break; + } + case "profiles-manage-profiles-button": { + this.manageProfiles(); + break; + } + case "profiles-create-profile-button": { + this.createNewProfile(); + break; + } + /* Menubar events - separated out to simplify telemetry */ + case "Profiles:CreateProfile": { + this.createNewProfile(); + break; + } + case "Profiles:ManageProfiles": { + this.manageProfiles(); + break; + } + case "Profiles:LaunchProfile": { + this.launchProfile(aEvent.sourceEvent); break; } } + /* Appmenu */ + if (aEvent.target.classList.contains("profile-item")) { + this.launchProfile(aEvent); + } }, + /** + * Draws the subpanel contents for the app menu. + */ async populateSubView() { let profiles = []; let currentProfile = null; @@ -156,14 +275,15 @@ var gProfiles = { profilesHeader.removeAttribute("style"); editButton.hidden = true; } else { - profilesHeader.style.backgroundColor = "var(--themeBg)"; + profilesHeader.style.backgroundColor = "var(--appmenu-profiles-theme-bg)"; editButton.hidden = false; } if (currentProfile && profiles.length > 1) { let subview = PanelMultiView.getViewNode(document, "PanelUI-profiles"); let { themeBg, themeFg } = currentProfile.theme; - subview.style.cssText = `--themeBg: ${themeBg}; --themeFg: ${themeFg};`; + subview.style.setProperty("--appmenu-profiles-theme-bg", themeBg); + subview.style.setProperty("--appmenu-profiles-theme-fg", themeFg); let headerText = PanelMultiView.getViewNode( document, @@ -175,7 +295,14 @@ var gProfiles = { document, "profile-icon-image" ); - currentProfileCard.style.cssText = `--themeFg: ${themeFg}; --themeBg: ${themeBg};`; + currentProfileCard.style.setProperty( + "--appmenu-profiles-theme-bg", + themeBg + ); + currentProfileCard.style.setProperty( + "--appmenu-profiles-theme-fg", + themeFg + ); let avatar = currentProfile.avatar; profileIconEl.style.listStyleImage = `url("chrome://browser/content/profiles/assets/80_${avatar}.svg")`; @@ -198,7 +325,8 @@ var gProfiles = { button.setAttribute("label", profile.name); button.className = "subviewbutton subviewbutton-iconic profile-item"; let { themeFg, themeBg } = profile.theme; - button.style.cssText = `--themeBg: ${themeBg}; --themeFg: ${themeFg};`; + button.style.setProperty("--appmenu-profiles-theme-bg", themeBg); + button.style.setProperty("--appmenu-profiles-theme-fg", themeFg); button.setAttribute( "image", `chrome://browser/content/profiles/assets/16_${profile.avatar}.svg` diff --git a/browser/base/content/browser-sets.inc b/browser/base/content/browser-sets.inc index 7ef9c29f14ad..493c593d6688 100644 --- a/browser/base/content/browser-sets.inc +++ b/browser/base/content/browser-sets.inc @@ -76,6 +76,9 @@ + + + diff --git a/browser/base/content/browser-sets.js b/browser/base/content/browser-sets.js index 94b106099b57..c4e16f99753a 100644 --- a/browser/base/content/browser-sets.js +++ b/browser/base/content/browser-sets.js @@ -205,6 +205,12 @@ document.addEventListener( case "Browser:OpenAboutContainers": openPreferences("paneContainers"); break; + // deliberate fallthrough + case "Profiles:CreateProfile": + case "Profiles:ManageProfiles": + case "Profiles:LaunchProfile": + gProfiles.handleCommand(event); + break; case "Tools:Search": BrowserSearch.webSearch(); break; diff --git a/browser/components/profiles/tests/browser/browser.toml b/browser/components/profiles/tests/browser/browser.toml index fc57b4de7a15..99d0bf56433a 100644 --- a/browser/components/profiles/tests/browser/browser.toml +++ b/browser/components/profiles/tests/browser/browser.toml @@ -9,10 +9,16 @@ prefs = [ ["browser_activate.js"] +["browser_appmenu_menuitem_updates.js"] +head = "../unit/head.js head.js" + ["browser_create_profile_page_test.js"] ["browser_edit_profile_test.js"] +["browser_menubar_profiles.js"] +head = "../unit/head.js head.js" + ["browser_notify_changes.js"] run-if = ["os != 'linux'"] # Linux clients cannot remote themselves. diff --git a/browser/components/profiles/tests/browser/browser_menubar_profiles.js b/browser/components/profiles/tests/browser/browser_menubar_profiles.js new file mode 100644 index 000000000000..3d88cd32fad4 --- /dev/null +++ b/browser/components/profiles/tests/browser/browser_menubar_profiles.js @@ -0,0 +1,64 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// TODO (bug 1928538): figure out what condition to wait for instead of setting +// an arbitrary timeout. +async function waitForUIUpdate() { + /* eslint-disable mozilla/no-arbitrary-setTimeout */ + await new Promise(resolve => setTimeout(resolve, 500)); +} + +add_task(async function test_pref_toggles_menu() { + registerCleanupFunction(async () => { + await SpecialPowers.popPrefEnv(); + }); + // The pref observer only fires on change, and browser.toml sets the pref to + // true, so we disable then re-enable the pref. + await SpecialPowers.pushPrefEnv({ + set: [["browser.profiles.enabled", false]], + }); + await SpecialPowers.pushPrefEnv({ + set: [["browser.profiles.enabled", true]], + }); + await waitForUIUpdate(); + let menu = document.getElementById("profiles-menu"); + Assert.equal(menu.hidden, false, "menu should be visible when preffed on"); + await SpecialPowers.popPrefEnv(); + await waitForUIUpdate(); + Assert.equal(menu.hidden, true, "menu should be hidden when preffed off"); +}); + +add_task(async function test_menu_contents_no_profiles() { + registerCleanupFunction(async () => { + await SpecialPowers.popPrefEnv(); + }); + await SpecialPowers.pushPrefEnv({ + set: [["browser.profiles.enabled", true]], + }); + let popup = document.getElementById("menu_ProfilesPopup"); + + // Uninit the service to simulate a user with no profiles. + SelectableProfileService.uninit(); + await waitForUIUpdate(); + + // Simulate opening the menu, as seen in browser_file_close_tabs.js. + let updated = new Promise(resolve => { + popup.addEventListener("popupshown", resolve, { once: true }); + }); + popup.dispatchEvent(new MouseEvent("popupshowing", { bubbles: true })); + popup.dispatchEvent(new MouseEvent("popupshown", { bubbles: true })); + await updated; + + let newProfileMenuItem = popup.querySelector("#menu_newProfile"); + ok(!!newProfileMenuItem, "should be a 'new profile' menu item"); + let manageProfilesMenuItem = popup.querySelector("#menu_manageProfiles"); + ok(!!manageProfilesMenuItem, "should be a 'manage profiles' menu item"); + let profileMenuItems = popup.querySelectorAll("menuitem[profileid]"); + Assert.equal( + profileMenuItems.length, + 0, + "should not be any profile items in the menu" + ); +}); diff --git a/browser/locales/en-US/browser/menubar.ftl b/browser/locales/en-US/browser/menubar.ftl index f65a345d6050..abb880d9019d 100644 --- a/browser/locales/en-US/browser/menubar.ftl +++ b/browser/locales/en-US/browser/menubar.ftl @@ -248,6 +248,15 @@ menu-bookmarks-other = menu-bookmarks-mobile = .label = Mobile Bookmarks +## Profiles Menu + +menu-profiles = + .label = Profiles +menu-profiles-manage-profiles = + .label = Manage profiles +menu-profiles-new-profile = + .label = New profile + ## Tools Menu menu-tools = diff --git a/browser/themes/shared/browser-shared.css b/browser/themes/shared/browser-shared.css index c80e568cfaa8..8f20ff9a43c4 100644 --- a/browser/themes/shared/browser-shared.css +++ b/browser/themes/shared/browser-shared.css @@ -358,6 +358,14 @@ body { display: none; } +/* Profiles menu */ + +.menuitem-iconic-profile { + -moz-context-properties: fill, stroke; + fill: var(--menu-profiles-theme-bg); + stroke: var(--menu-profiles-theme-fg); +} + /* Navigation toolbar */ #nav-bar { diff --git a/browser/themes/shared/customizableui/panelUI-shared.css b/browser/themes/shared/customizableui/panelUI-shared.css index 6d542093f0bd..6fef19331da0 100644 --- a/browser/themes/shared/customizableui/panelUI-shared.css +++ b/browser/themes/shared/customizableui/panelUI-shared.css @@ -1188,12 +1188,13 @@ panelview .toolbarbutton-1, justify-content: space-between; } -/* Set the --themeBg and --themeFg variables inline on the toolbarbutton */ +/* Set the --appmenu-profiles-theme-bg and --appmenu-profiles-theme-fg + * variables inline on the toolbarbutton */ #appMenu-profiles-button.subviewbutton-iconic > .toolbarbutton-icon, .subviewbutton-iconic.profile-item > .toolbarbutton-icon { -moz-context-properties: fill, stroke; - fill: var(--themeBg); - stroke: var(--themeFg); + fill: var(--appmenu-profiles-theme-bg); + stroke: var(--appmenu-profiles-theme-fg); } /* Adjust the Zoom toolbaritem padding to have its height the same as other toolbarbuttons, @@ -2221,8 +2222,8 @@ radiogroup:focus-visible > .subviewradio[focused="true"] { width: 75px; height: 75px; -moz-context-properties: fill, stroke; - fill: var(--themeBg); - stroke: var(--themeFg); + fill: var(--appmenu-profiles-theme-bg); + stroke: var(--appmenu-profiles-theme-fg); border: 4px solid var(--panel-background); border-radius: 40px; } @@ -2234,7 +2235,7 @@ radiogroup:focus-visible > .subviewradio[focused="true"] { } #half-height-profile { - background: var(--themeBg); + background: var(--appmenu-profiles-theme-bg); width: 100%; height: 50%; z-index: -1;