Bug 1391948 - Don't use checkboxes for the "View Bookmarks Toolbar" and "View Bookmarks Sidebar" items in the Bookmarks button menu. r=mak
MozReview-Commit-ID: 2P3yV6xHLxi
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -1281,27 +1281,16 @@ var BookmarkingUI = {
_getFormattedTooltip(strId) {
let args = [];
let shortcut = document.getElementById(this.BOOKMARK_BUTTON_SHORTCUT);
if (shortcut)
args.push(ShortcutUtils.prettifyShortcut(shortcut));
return gNavigatorBundle.getFormattedString(strId, args);
},
- /**
- * The popup contents must be updated when the user customizes the UI, or
- * changes the personal toolbar collapsed status. In such a case, any needed
- * change should be handled in the popupshowing helper, for performance
- * reasons.
- */
- _popupNeedsUpdate: true,
- onToolbarVisibilityChange: function BUI_onToolbarVisibilityChange() {
- this._popupNeedsUpdate = true;
- },
-
onPopupShowing: function BUI_onPopupShowing(event) {
// Don't handle events for submenus.
if (event.target != event.currentTarget)
return;
// On non-photon, this code should never be reached. However, if you click
// the outer button's border, some cpp code for the menu button's XBL
// binding decides to open the popup even though the dropmarker is invisible.
@@ -1323,33 +1312,31 @@ var BookmarkingUI = {
event.preventDefault();
widget.node.removeAttribute("closemenu");
PlacesCommandHook.showPlacesOrganizer("BookmarksMenu");
return;
}
this._initMobileBookmarks(document.getElementById("BMB_mobileBookmarks"));
- if (!this._popupNeedsUpdate)
- return;
- this._popupNeedsUpdate = false;
+ this.selectLabel("BMB_viewBookmarksSidebar",
+ SidebarUI.currentID == "viewBookmarksSidebar");
+ this.selectLabel("BMB_viewBookmarksToolbar",
+ !document.getElementById("PersonalToolbar").collapsed);
+ },
- let popup = event.target;
- let getPlacesAnonymousElement =
- aAnonId => document.getAnonymousElementByAttribute(popup.parentNode,
- "placesanonid",
- aAnonId);
+ selectLabel(elementId, visible) {
+ let element = document.getElementById(elementId);
+ element.setAttribute("label", element.getAttribute(visible ? "label-hide"
+ : "label-show"));
+ },
- let viewToolbarMenuitem = getPlacesAnonymousElement("view-toolbar");
- if (viewToolbarMenuitem) {
- // Update View bookmarks toolbar checkbox menuitem.
- viewToolbarMenuitem.classList.add("subviewbutton");
- let personalToolbar = document.getElementById("PersonalToolbar");
- viewToolbarMenuitem.setAttribute("checked", !personalToolbar.collapsed);
- }
+ toggleBookmarksToolbar() {
+ CustomizableUI.setToolbarVisibility("PersonalToolbar",
+ document.getElementById("PersonalToolbar").collapsed);
},
attachPlacesView(event, node) {
// If the view is already there, bail out early.
if (node.parentNode._placesView)
return;
new PlacesMenu(event, "place:folder=BOOKMARKS_MENU", {
@@ -1432,17 +1419,16 @@ var BookmarkingUI = {
if (!this._isCustomizing) {
this._uninitView();
}
},
onCustomizeEnd: function BUI_customizeEnd(aWindow) {
if (aWindow == window) {
this._isCustomizing = false;
- this.onToolbarVisibilityChange();
}
},
init() {
CustomizableUI.addListener(this);
if (Services.prefs.getBoolPref("toolkit.cosmeticAnimations.enabled")) {
let starButtonBox = document.getElementById("star-button-box");
@@ -1643,50 +1629,24 @@ var BookmarkingUI = {
onPanelMenuViewHiding: function BUI_onViewHiding(aEvent) {
this._panelMenuView.uninit();
delete this._panelMenuView;
aEvent.target.removeEventListener("ViewHiding", this);
},
showBookmarkingTools(triggerNode) {
- const panelID = "PanelUI-bookmarkingTools";
- let viewNode = document.getElementById(panelID);
- for (let button of [...viewNode.getElementsByTagName("toolbarbutton")]) {
- let update = true;
- switch (button.id) {
- case "panelMenu_toggleBookmarksMenu":
- let placement = CustomizableUI.getPlacementOfWidget(this.BOOKMARK_BUTTON_ID);
- button.setAttribute("checked", !!placement && placement.area == CustomizableUI.AREA_NAVBAR);
- break;
- case "panelMenu_viewBookmarksSidebar":
- button.setAttribute("checked", SidebarUI.currentID == "viewBookmarksSidebar");
- break;
- case "panelMenu_viewBookmarksToolbar":
- let toolbar = document.getElementById("PersonalToolbar");
- // This is an actual toolbarbutton[type=checkbox], and its checked
- // attribute will get added/removed by the binding when clicked.
- // Setting the attribute to 'false' breaks showing the toolbar,
- // because the binding removes the attribute instead of setting it
- // to 'true' when clicked.
- if (toolbar.getAttribute("collapsed") != "true") {
- button.setAttribute("checked", "true");
- } else {
- button.removeAttribute("checked");
- }
- break;
- default:
- update = false;
- break;
- }
- if (update) {
- updateToggleControlLabel(button);
- }
- }
- PanelUI.showSubView(panelID, triggerNode);
+ let placement = CustomizableUI.getPlacementOfWidget(this.BOOKMARK_BUTTON_ID);
+ this.selectLabel("panelMenu_toggleBookmarksMenu",
+ placement && placement.area == CustomizableUI.AREA_NAVBAR);
+ this.selectLabel("panelMenu_viewBookmarksSidebar",
+ SidebarUI.currentID == "viewBookmarksSidebar");
+ this.selectLabel("panelMenu_viewBookmarksToolbar",
+ !document.getElementById("PersonalToolbar").collapsed);
+ PanelUI.showSubView("PanelUI-bookmarkingTools", triggerNode);
},
toggleMenuButtonInToolbar(triggerNode) {
let placement = CustomizableUI.getPlacementOfWidget(this.BOOKMARK_BUTTON_ID);
const area = CustomizableUI.AREA_NAVBAR;
if (!placement) {
// Button is in the palette, so we can move it to the navbar.
let pos;
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -5598,18 +5598,16 @@ function setToolbarVisibility(toolbar, i
detail: {
visible: isVisible
},
bubbles: true
};
let event = new CustomEvent("toolbarvisibilitychange", eventParams);
toolbar.dispatchEvent(event);
- BookmarkingUI.onToolbarVisibilityChange();
-
if (toolbar.getAttribute("type") == "menubar" && CustomizationHandler.isCustomizing()) {
gCustomizeMode._updateDragSpaceCheckbox();
}
}
function updateToggleControlLabel(control) {
if (!control.hasAttribute("label-checked")) {
return;
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -1105,21 +1105,19 @@
onmouseup="BookmarksEventHandler.onMouseUp(event);"
oncommand="BookmarksEventHandler.onCommand(event);"
onclick="BookmarksEventHandler.onClick(event, this.parentNode._placesView);"
onpopupshowing="BookmarkingUI.onPopupShowing(event);
BookmarkingUI.attachPlacesView(event, this);"
tooltip="bhTooltip" popupsinherittooltip="true">
<menuitem id="BMB_viewBookmarksSidebar"
class="subviewbutton"
- label="&viewBookmarksSidebar2.label;"
- type="checkbox"
- oncommand="SidebarUI.toggle('viewBookmarksSidebar');">
- <observes element="viewBookmarksSidebar" attribute="checked"/>
- </menuitem>
+ label-show="&viewBookmarksSidebar2.label;"
+ label-hide="&hideBookmarksSidebar.label;"
+ oncommand="SidebarUI.toggle('viewBookmarksSidebar');"/>
<!-- NB: temporary solution for bug 985024, this should go away soon. -->
<menuitem id="BMB_bookmarksShowAllTop"
class="menuitem-iconic subviewbutton"
label="&showAllBookmarks2.label;"
command="Browser:ShowAllBookmarks"
key="manBookmarkKb"/>
<menuseparator/>
<menu id="BMB_bookmarksToolbar"
@@ -1128,21 +1126,20 @@
container="true">
<menupopup id="BMB_bookmarksToolbarPopup"
placespopup="true"
context="placesContext"
onpopupshowing="if (!this.parentNode._placesView)
new PlacesMenu(event, 'place:folder=TOOLBAR',
PlacesUIUtils.getViewForNode(this.parentNode.parentNode).options);">
<menuitem id="BMB_viewBookmarksToolbar"
- placesanonid="view-toolbar"
- toolbarId="PersonalToolbar"
- type="checkbox"
- oncommand="onViewToolbarCommand(event)"
- label="&viewBookmarksToolbar.label;"/>
+ class="subviewbutton"
+ label-show="&viewBookmarksToolbar.label;"
+ label-hide="&hideBookmarksToolbar.label;"
+ oncommand="BookmarkingUI.toggleBookmarksToolbar();"/>
<menuseparator/>
<!-- Bookmarks toolbar items -->
</menupopup>
</menu>
<menu id="BMB_unsortedBookmarks"
class="menu-iconic bookmark-item subviewbutton"
label="&bookmarksMenuButton.other.label;"
container="true">
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -655,36 +655,31 @@
<!-- Recent Highlights will go here -->
</toolbaritem>
</vbox>
</panelview>
<panelview id="PanelUI-bookmarkingTools" class="PanelUI-subView">
<vbox class="panel-subview-body">
<toolbarbutton id="panelMenu_toggleBookmarksMenu"
- label="&addBookmarksMenu.label;"
- label-checked="&removeBookmarksMenu.label;"
class="subviewbutton subviewbutton-iconic"
- oncommand="BookmarkingUI.toggleMenuButtonInToolbar(this); PanelUI.hide();"/>
+ label-show="&addBookmarksMenu.label;"
+ label-hide="&removeBookmarksMenu.label;"
+ oncommand="BookmarkingUI.toggleMenuButtonInToolbar(this);"/>
<toolbarbutton id="panelMenu_viewBookmarksSidebar"
- label="&viewBookmarksSidebar2.label;"
- label-checked="&hideBookmarksSidebar.label;"
class="subviewbutton subviewbutton-iconic"
+ label-show="&viewBookmarksSidebar2.label;"
+ label-hide="&hideBookmarksSidebar.label;"
key="viewBookmarksSidebarKb"
- oncommand="SidebarUI.toggle('viewBookmarksSidebar', this); PanelUI.hide();">
- <observes element="viewBookmarksSidebar" attribute="checked"/>
- </toolbarbutton>
+ oncommand="SidebarUI.toggle('viewBookmarksSidebar', this);"/>
<toolbarbutton id="panelMenu_viewBookmarksToolbar"
class="subviewbutton subviewbutton-iconic"
- placesanonid="view-toolbar"
- toolbarId="PersonalToolbar"
- type="checkbox"
- oncommand="onViewToolbarCommand(event)"
- label="&viewBookmarksToolbar.label;"
- label-checked="&hideBookmarksToolbar.label;"/>
+ label-show="&viewBookmarksToolbar.label;"
+ label-hide="&hideBookmarksToolbar.label;"
+ oncommand="BookmarkingUI.toggleBookmarksToolbar();"/>
</vbox>
</panelview>
</panelmultiview>
</panel>
<panel id="downloads-button-autohide-panel"
role="group"
type="arrow"