From 4c430fb4a6726a0816deb45af93f6af810004d9f Mon Sep 17 00:00:00 2001 From: Harry Twyford Date: Tue, 13 Apr 2021 13:24:59 +0000 Subject: [PATCH] Bug 1704474 - Remove pin/unpin page action context menu items. r=adw,fluent-reviewers,extension-reviewers,flod,zombie The bug calls for these items to be hidden with JS, but they were going to be removed anyways post-Proton. The removal of some subtests in browser/base/content/test/pageActions tests is consistent with [this comment](https://searchfox.org/mozilla-central/rev/d9f6cded535d202a9ade4a530e653e659bcb5bbd/browser/base/content/test/pageActions/browser.ini#7), which says that were are removing that test coverage post-Proton anyways. Differential Revision: https://phabricator.services.mozilla.com/D111713 --- browser/base/content/browser-pageActions.js | 30 ----- browser/base/content/browser.css | 22 ---- browser/base/content/browser.xhtml | 13 -- .../pageActions/browser_page_action_menu.js | 123 ------------------ .../browser_page_action_menu_share_mac.js | 58 --------- .../test/browser/browser_ext_menus.js | 10 +- browser/locales/en-US/browser/browser.ftl | 4 - 7 files changed, 4 insertions(+), 256 deletions(-) diff --git a/browser/base/content/browser-pageActions.js b/browser/base/content/browser-pageActions.js index c35a5ae7f5d4c..f777d8afbe6d1 100644 --- a/browser/base/content/browser-pageActions.js +++ b/browser/base/content/browser-pageActions.js @@ -980,18 +980,6 @@ var BrowserPageActions = { } this._contextAction = action; - let state; - if (this._contextAction._isMozillaAction) { - state = this._contextAction.pinnedToUrlbar - ? "builtInPinned" - : "builtInUnpinned"; - } else { - state = this._contextAction.pinnedToUrlbar - ? "extensionPinned" - : "extensionUnpinned"; - } - popup.setAttribute("state", state); - let removeExtension = popup.querySelector(".removeExtensionItem"); let { extensionID } = this._contextAction; let addon = extensionID && (await AddonManager.getAddonByID(extensionID)); @@ -1003,24 +991,6 @@ var BrowserPageActions = { } }, - /** - * Call this from the menu item in the context menu that toggles pinning. - */ - togglePinningForContextAction() { - if (!this._contextAction) { - return; - } - let action = this._contextAction; - this._contextAction = null; - - action.pinnedToUrlbar = !action.pinnedToUrlbar; - BrowserUsageTelemetry.recordWidgetChange( - action.id, - action.pinnedToUrlbar ? "page-action-buttons" : null, - "pageaction-context" - ); - }, - /** * Call this from the menu item in the context menu that opens about:addons. */ diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css index 442602163ab68..06a33df15d25a 100644 --- a/browser/base/content/browser.css +++ b/browser/base/content/browser.css @@ -1412,28 +1412,6 @@ toolbarpaletteitem > #stop-reload-button { } } -/* Page action context menu */ -#pageActionContextMenu > .pageActionContextMenuItem { - visibility: collapse; -} - -#pageActionContextMenu[state=extensionPinned] > .pageActionContextMenuItem.manageExtensionItem, -#pageActionContextMenu[state=extensionUnpinned] > .pageActionContextMenuItem.manageExtensionItem, -#pageActionContextMenu[state=extensionPinned] > .pageActionContextMenuItem.removeExtensionItem, -#pageActionContextMenu[state=extensionUnpinned] > .pageActionContextMenuItem.removeExtensionItem { - visibility: visible; -} - -@media not (-moz-proton) { -/* These pin/unpin menu options should be removed after Proton is released */ -#pageActionContextMenu[state=builtInPinned] > .pageActionContextMenuItem.builtInPinned, -#pageActionContextMenu[state=builtInUnpinned] > .pageActionContextMenuItem.builtInUnpinned, -#pageActionContextMenu[state=extensionPinned] > .pageActionContextMenuItem.extensionPinned, -#pageActionContextMenu[state=extensionUnpinned] > .pageActionContextMenuItem.extensionUnpinned { - visibility: visible; -} -} /*** !proton ***/ - /* Print pending */ .printSettingsBrowser { width: 250px !important; diff --git a/browser/base/content/browser.xhtml b/browser/base/content/browser.xhtml index 701a6bf1a6efd..895a59956abc2 100644 --- a/browser/base/content/browser.xhtml +++ b/browser/base/content/browser.xhtml @@ -603,19 +603,6 @@ - - - - - diff --git a/browser/base/content/test/pageActions/browser_page_action_menu.js b/browser/base/content/test/pageActions/browser_page_action_menu.js index e04084c055280..699b5486ebeaf 100644 --- a/browser/base/content/test/pageActions/browser_page_action_menu.js +++ b/browser/base/content/test/pageActions/browser_page_action_menu.js @@ -1049,122 +1049,6 @@ add_task(async function sendToDevice_inUrlbar() { }); }); -add_task(async function contextMenu() { - // Open an actionable page so that the main page action button appears. - let url = "http://example.com/"; - await BrowserTestUtils.withNewTab(url, async () => { - // Open the panel and then open the context menu on the bookmark button. - await promisePageActionPanelOpen(); - let bookmarkButton = document.getElementById("pageAction-panel-bookmark"); - let contextMenuPromise = promisePanelShown("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(bookmarkButton, { - type: "contextmenu", - button: 2, - }); - await contextMenuPromise; - - // The context menu should show the "remove" item. Click it. - let menuItems = collectContextMenuItems(); - Assert.equal(menuItems.length, 1, "Context menu has one child"); - Assert.equal( - menuItems[0].label, - "Remove from Address Bar", - "Context menu is in the 'remove' state" - ); - contextMenuPromise = promisePanelHidden("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(menuItems[0], {}); - await contextMenuPromise; - - // The action should be removed from the urlbar. In this case, the bookmark - // star, the node in the urlbar should be hidden. - let starButtonBox = document.getElementById("star-button-box"); - await TestUtils.waitForCondition(() => { - return starButtonBox.hidden; - }, "Waiting for star button to become hidden"); - - // Open the context menu again on the bookmark button. (The page action - // panel remains open.) - contextMenuPromise = promisePanelShown("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(bookmarkButton, { - type: "contextmenu", - button: 2, - }); - await contextMenuPromise; - - // The context menu should show the "add" item. Click it. - menuItems = collectContextMenuItems(); - Assert.equal(menuItems.length, 1, "Context menu has one child"); - Assert.equal( - menuItems[0].label, - "Add to Address Bar", - "Context menu is in the 'add' state" - ); - contextMenuPromise = promisePanelHidden("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(menuItems[0], {}); - await contextMenuPromise; - - // The action should be added to the urlbar. - await TestUtils.waitForCondition(() => { - return !starButtonBox.hidden; - }, "Waiting for star button to become unhidden"); - - // Open the context menu on the bookmark star in the urlbar. - contextMenuPromise = promisePanelShown("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(starButtonBox, { - type: "contextmenu", - button: 2, - }); - await contextMenuPromise; - - // The context menu should show the "remove" item. Click it. - menuItems = collectContextMenuItems(); - Assert.equal(menuItems.length, 1, "Context menu has one child"); - Assert.equal( - menuItems[0].label, - "Remove from Address Bar", - "Context menu is in the 'remove' state" - ); - contextMenuPromise = promisePanelHidden("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(menuItems[0], {}); - await contextMenuPromise; - - // The action should be removed from the urlbar. - await TestUtils.waitForCondition(() => { - return starButtonBox.hidden; - }, "Waiting for star button to become hidden"); - - // Finally, add the bookmark star back to the urlbar so that other tests - // that rely on it are OK. - await promisePageActionPanelOpen(); - contextMenuPromise = promisePanelShown("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(bookmarkButton, { - type: "contextmenu", - button: 2, - }); - await contextMenuPromise; - - menuItems = collectContextMenuItems(); - Assert.equal(menuItems.length, 1, "Context menu has one child"); - Assert.equal( - menuItems[0].label, - "Add to Address Bar", - "Context menu is in the 'add' state" - ); - contextMenuPromise = promisePanelHidden("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(menuItems[0], {}); - await contextMenuPromise; - await TestUtils.waitForCondition(() => { - return !starButtonBox.hidden; - }, "Waiting for star button to become unhidden"); - }); - - // urlbar tests that run after this one can break if the mouse is left over - // the area where the urlbar popup appears, which seems to happen due to the - // above synthesized mouse events. Move it over the urlbar. - EventUtils.synthesizeMouseAtCenter(gURLBar.textbox, { type: "mousemove" }); - gURLBar.focus(); -}); - function promiseSyncReady() { let service = Cc["@mozilla.org/weave/service;1"].getService(Ci.nsISupports) .wrappedJSObject; @@ -1215,10 +1099,3 @@ function checkSendToDeviceItems(expectedItems, forUrlbar = false) { } } } - -function collectContextMenuItems() { - let contextMenu = document.getElementById("pageActionContextMenu"); - return Array.prototype.filter.call(contextMenu.children, node => { - return window.getComputedStyle(node).visibility == "visible"; - }); -} diff --git a/browser/base/content/test/pageActions/browser_page_action_menu_share_mac.js b/browser/base/content/test/pageActions/browser_page_action_menu_share_mac.js index 1b74d800d3e67..c1b38b2a8ce90 100644 --- a/browser/base/content/test/pageActions/browser_page_action_menu_share_mac.js +++ b/browser/base/content/test/pageActions/browser_page_action_menu_share_mac.js @@ -88,64 +88,6 @@ add_task(async function shareURL() { }); }); -add_task(async function shareURLAddressBar() { - await BrowserTestUtils.withNewTab(URL, async () => { - // Open pageAction panel - await promisePageActionPanelOpen(); - - // Right click the Share button - let contextMenuPromise = promisePanelShown("pageActionContextMenu"); - let shareURLButton = document.getElementById("pageAction-panel-shareURL"); - EventUtils.synthesizeMouseAtCenter(shareURLButton, { - type: "contextmenu", - button: 2, - }); - await contextMenuPromise; - - // Click "Add to Address Bar" - contextMenuPromise = promisePanelHidden("pageActionContextMenu"); - let ctxMenuButton = document.querySelector( - "#pageActionContextMenu .pageActionContextMenuItem" - ); - EventUtils.synthesizeMouseAtCenter(ctxMenuButton, {}); - await contextMenuPromise; - - // Wait for the Share button to be added - await TestUtils.waitForCondition(() => { - return document.getElementById("pageAction-urlbar-shareURL"); - }, "Waiting for the share url button to be added to url bar"); - - // Press the Share button - let shareButton = document.getElementById("pageAction-urlbar-shareURL"); - let viewPromise = promisePageActionPanelShown(); - EventUtils.synthesizeMouseAtCenter(shareButton, {}); - await viewPromise; - - // Ensure we have share providers - let panel = document.getElementById( - "pageAction-urlbar-shareURL-subview-body" - ); - // We should see 1 receiver and one extra node for the "More..." button - Assert.equal(panel.children.length, 2, "Has correct share receivers"); - - // Remove the Share URL button from the Address bar so we dont interfere - // with future tests - contextMenuPromise = promisePanelShown("pageActionContextMenu"); - EventUtils.synthesizeMouseAtCenter(shareButton, { - type: "contextmenu", - button: 2, - }); - await contextMenuPromise; - - contextMenuPromise = promisePanelHidden("pageActionContextMenu"); - ctxMenuButton = document.querySelector( - "#pageActionContextMenu .pageActionContextMenuItem" - ); - EventUtils.synthesizeMouseAtCenter(ctxMenuButton, {}); - await contextMenuPromise; - }); -}); - add_task(async function openSharingPreferences() { await BrowserTestUtils.withNewTab(URL, async () => { // Open the panel. diff --git a/browser/components/extensions/test/browser/browser_ext_menus.js b/browser/components/extensions/test/browser/browser_ext_menus.js index e15fb579f7db6..cb3e7d0786156 100644 --- a/browser/components/extensions/test/browser/browser_ext_menus.js +++ b/browser/components/extensions/test/browser/browser_ext_menus.js @@ -218,13 +218,11 @@ add_task(async function test_hiddenPageActionContextMenu() { return window.getComputedStyle(node).visibility == "visible"; }); - is(menuItems.length, 4, "Correct number of children"); - const [dontShowItem, separator, manageItem, removeItem] = menuItems; + is(menuItems.length, 2, "Correct number of children"); + const [manageItem, removeItem] = menuItems; - is(dontShowItem.label, "Remove from Address Bar", "Correct first child"); - is(separator.tagName, "menuseparator", "Correct second child"); - is(manageItem.label, "Manage Extension\u2026", "Correct third child"); - is(removeItem.label, "Remove Extension", "Correct fourth child"); + is(manageItem.label, "Manage Extension\u2026", "Correct first child"); + is(removeItem.label, "Remove Extension", "Correct second child"); await closeChromeContextMenu(menu.id); await closeChromeContextMenu(BrowserPageActions.panelNode.id); diff --git a/browser/locales/en-US/browser/browser.ftl b/browser/locales/en-US/browser/browser.ftl index 9994aba801fc2..d8efbe6290df9 100644 --- a/browser/locales/en-US/browser/browser.ftl +++ b/browser/locales/en-US/browser/browser.ftl @@ -160,12 +160,8 @@ urlbar-star-add-bookmark = ## Page Action Context Menu -page-action-add-to-urlbar = - .label = Add to Address Bar page-action-manage-extension = .label = Manage Extension… -page-action-remove-from-urlbar = - .label = Remove from Address Bar page-action-remove-extension = .label = Remove Extension