Skip to content

Commit

Permalink
Bug 1686526, hide sidebar button in proton toolbar r=mstriemer
Browse files Browse the repository at this point in the history
  • Loading branch information
Emma Malysz committed Feb 24, 2021
1 parent f50764c commit e2feec6
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ module.exports = {
"browser/components/customizableui/test/browser_989338_saved_placements_not_resaved.js",
"browser/components/customizableui/test/browser_currentset_post_reset.js",
"browser/components/customizableui/test/browser_panel_keyboard_navigation.js",
"browser/components/customizableui/test/browser_proton_toolbar_hide_home_and_library_buttons.js",
"browser/components/customizableui/test/browser_proton_toolbar_hide_toolbarbuttons.js",
"browser/components/enterprisepolicies/tests/browser/browser_policies_setAndLockPref_API.js",
"browser/components/enterprisepolicies/tests/browser/head.js",
"browser/components/enterprisepolicies/tests/xpcshell/head.js",
Expand Down
15 changes: 13 additions & 2 deletions browser/components/customizableui/CustomizableUI.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const kPrefAutoHideDownloadsButton = "browser.download.autohideButton";
const kPrefProtonToolbarVersion = "browser.proton.toolbar.version";
const kPrefHomeButtonUsed = "browser.engagement.home-button.has-used";
const kPrefLibraryButtonUsed = "browser.engagement.library-button.has-used";
const kPrefSidebarButtonUsed = "browser.engagement.sidebar-button.has-used";

const kExpectedWindowURL = AppConstants.BROWSER_CHROME_URL;

Expand Down Expand Up @@ -263,7 +264,7 @@ var CustomizableUIInternal = {
"downloads-button",
gProtonToolbarEnabled ? null : "library-button",
AppConstants.MOZ_DEV_EDITION ? "developer-button" : null,
"sidebar-button",
gProtonToolbarEnabled ? null : "sidebar-button",
"fxa-toolbar-menu-button",
].filter(name => name);

Expand Down Expand Up @@ -602,7 +603,7 @@ var CustomizableUIInternal = {
},

_updateForNewProtonVersion() {
const VERSION = 2;
const VERSION = 3;
let currentVersion = Services.prefs.getIntPref(
kPrefProtonToolbarVersion,
0
Expand Down Expand Up @@ -642,6 +643,16 @@ var CustomizableUIInternal = {
}
}

// Remove the library button if it hasn't been used
if (currentVersion < 3) {
if (
placements.includes("sidebar-button") &&
!Services.prefs.getBoolPref(kPrefSidebarButtonUsed)
) {
placements.splice(placements.indexOf("sidebar-button"), 1);
}
}

Services.prefs.setIntPref(kPrefProtonToolbarVersion, VERSION);
},

Expand Down
2 changes: 1 addition & 1 deletion browser/components/customizableui/test/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ skip-if = os == "mac"
skip-if = (verify && (os == 'linux' || os == 'mac'))
tags = fullscreen
[browser_panelUINotifications_multiWindow.js]
[browser_proton_toolbar_hide_home_and_library_buttons.js]
[browser_proton_toolbar_hide_toolbarbuttons.js]
[browser_remove_customized_specials.js]
[browser_reset_builtin_widget_currentArea.js]
[browser_switch_to_customize_mode.js]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@ const kPrefProtonToolbarEnabled = "browser.proton.toolbar.enabled";
const kPrefProtonToolbarVersion = "browser.proton.toolbar.version";
const kPrefHomeButtonUsed = "browser.engagement.home-button.has-used";
const kPrefLibraryButtonUsed = "browser.engagement.library-button.has-used";
const kPrefSidebarButtonUsed = "browser.engagement.sidebar-button.has-used";

async function testToolbarButtons(
shouldRemoveHomeButton,
shouldRemoveLibraryButton,
shouldUpdateVersion
) {
async function testToolbarButtons(aActions) {
let {
shouldRemoveHomeButton,
shouldRemoveLibraryButton,
shouldRemoveSidebarButton,
shouldUpdateVersion,
} = aActions;
const defaultPlacements = [
"back-button",
"forward-button",
Expand Down Expand Up @@ -58,6 +61,7 @@ async function testToolbarButtons(
let navbarPlacements = CustomizableUIBSPass.gSavedState.placements["nav-bar"];
let includesHomeButton = navbarPlacements.includes("home-button");
let includesLibraryButton = navbarPlacements.includes("library-button");
let includesSidebarButton = navbarPlacements.includes("sidebar-button");

Assert.equal(
!includesHomeButton,
Expand All @@ -69,6 +73,11 @@ async function testToolbarButtons(
shouldRemoveLibraryButton,
"Correctly handles library button"
);
Assert.equal(
!includesSidebarButton,
shouldRemoveSidebarButton,
"Correctly handles sidebar button"
);

let toolbarVersion = Services.prefs.getIntPref(kPrefProtonToolbarVersion);
if (shouldUpdateVersion) {
Expand Down Expand Up @@ -96,40 +105,53 @@ add_task(async function testButtonRemoval() {
set: [
[kPrefHomeButtonUsed, false],
[kPrefLibraryButtonUsed, false],
[kPrefSidebarButtonUsed, false],
],
});

let tests = [
// Proton enabled without home and library engagement
{
prefs: [[kPrefProtonToolbarEnabled, true]],
shouldRemoveHomeButton: true,
shouldRemoveLibraryButton: true,
shouldUpdateVersion: true,
actions: {
shouldRemoveHomeButton: true,
shouldRemoveLibraryButton: true,
shouldRemoveSidebarButton: true,
shouldUpdateVersion: true,
},
},
// Proton enabled with home engagement
{
prefs: [
[kPrefProtonToolbarEnabled, true],
[kPrefHomeButtonUsed, true],
],
shouldRemoveHomeButton: false,
shouldRemoveLibraryButton: true,
shouldUpdateVersion: true,
actions: {
shouldRemoveHomeButton: false,
shouldRemoveLibraryButton: true,
shouldRemoveSidebarButton: true,
shouldUpdateVersion: true,
},
},
// Proton disabled
{
prefs: [[kPrefProtonToolbarEnabled, false]],
shouldRemoveHomeButton: false,
shouldRemoveLibraryButton: false,
shouldUpdateVersion: false,
actions: {
shouldRemoveHomeButton: false,
shouldRemoveLibraryButton: false,
shouldRemoveSidebarButton: false,
shouldUpdateVersion: false,
},
},
// Proton enabled with custom homepage
{
prefs: [[kPrefProtonToolbarEnabled, true]],
shouldRemoveHomeButton: false,
shouldRemoveLibraryButton: true,
shouldUpdateVersion: true,
actions: {
shouldRemoveHomeButton: false,
shouldRemoveLibraryButton: true,
shouldRemoveSidebarButton: true,
shouldUpdateVersion: true,
},
async fn() {
HomePage.safeSet("https://example.com");
},
Expand All @@ -140,9 +162,25 @@ add_task(async function testButtonRemoval() {
[kPrefProtonToolbarEnabled, true],
[kPrefLibraryButtonUsed, true],
],
shouldRemoveHomeButton: true,
shouldRemoveLibraryButton: false,
shouldUpdateVersion: true,
actions: {
shouldRemoveHomeButton: true,
shouldRemoveLibraryButton: false,
shouldRemoveSidebarButton: true,
shouldUpdateVersion: true,
},
},
// Proton enabled with sidebar engagement
{
prefs: [
[kPrefProtonToolbarEnabled, true],
[kPrefSidebarButtonUsed, true],
],
actions: {
shouldRemoveHomeButton: true,
shouldRemoveLibraryButton: true,
shouldRemoveSidebarButton: false,
shouldUpdateVersion: true,
},
},
];

Expand All @@ -153,11 +191,7 @@ add_task(async function testButtonRemoval() {
if (test.fn) {
await test.fn();
}
testToolbarButtons(
test.shouldRemoveHomeButton,
test.shouldRemoveLibraryButton,
test.shouldUpdateVersion
);
testToolbarButtons(test.actions);
HomePage.reset();
await SpecialPowers.popPrefEnv();
}
Expand Down Expand Up @@ -199,6 +233,10 @@ add_task(async function testNullSavedState() {
!navbarPlacements.includes("library-button"),
"Library button isn't included by default"
);
Assert.ok(
!navbarPlacements.includes("sidebar-button"),
"Sidebar button isn't included by default"
);

// Cleanup
CustomizableUIBSPass.gSavedState = oldState;
Expand Down

0 comments on commit e2feec6

Please sign in to comment.