Skip to content

Commit

Permalink
Bug 1704474 - Remove pin/unpin page action context menu items. r=adw,…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@htwyford
htwyford committed Apr 13, 2021
1 parent 2afeed0 commit 4c430fb
Show file tree
Hide file tree
Showing 7 changed files with 4 additions and 256 deletions.
30 changes: 0 additions & 30 deletions browser/base/content/browser-pageActions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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.
*/
Expand Down
22 changes: 0 additions & 22 deletions browser/base/content/browser.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 0 additions & 13 deletions browser/base/content/browser.xhtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,19 +603,6 @@

<menupopup id="pageActionContextMenu"
onpopupshowing="BrowserPageActions.onContextMenuShowing(event, this);">
<menuitem class="pageActionContextMenuItem builtInUnpinned"
oncommand="BrowserPageActions.togglePinningForContextAction();"
data-l10n-id="page-action-add-to-urlbar"/>
<menuitem class="pageActionContextMenuItem builtInPinned"
oncommand="BrowserPageActions.togglePinningForContextAction();"
data-l10n-id="page-action-remove-from-urlbar"/>
<menuitem class="pageActionContextMenuItem extensionUnpinned"
oncommand="BrowserPageActions.togglePinningForContextAction();"
data-l10n-id="page-action-add-to-urlbar"/>
<menuitem class="pageActionContextMenuItem extensionPinned"
oncommand="BrowserPageActions.togglePinningForContextAction();"
data-l10n-id="page-action-remove-from-urlbar"/>
<menuseparator class="pageActionContextMenuItem extensionPinned extensionUnpinned"/>
<menuitem class="pageActionContextMenuItem extensionPinned extensionUnpinned manageExtensionItem"
oncommand="BrowserPageActions.openAboutAddonsForContextAction();"
data-l10n-id="page-action-manage-extension"/>
Expand Down
123 changes: 0 additions & 123 deletions browser/base/content/test/pageActions/browser_page_action_menu.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";
});
}
58 changes: 0 additions & 58 deletions browser/base/content/test/pageActions/browser_page_action_menu_share_mac.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 4 additions & 6 deletions browser/components/extensions/test/browser/browser_ext_menus.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions browser/locales/en-US/browser/browser.ftl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4c430fb

Please sign in to comment.