Bug 592900 - Update the bookmarks menu from the nav toolbar to match changes in the Firefox menu. r=dolske a=blocking

browser/base/content/browser-places.js

@@ -1153,39 +1153,49 @@ let BookmarksMenuButton = {
     // handled in the onPopupShowing handler, so it does not hit Ts.
   },
 
-  _popupInitialized: false,
-  _popupNeedsUpdating: true,
+  _popupNeedsUpdate: {},
   onPopupShowing: function BMB_onPopupShowing(event) {
-    if (!this._popupNeedsUpdating)
+    // Don't handle events for submenus.
+    if (event.target != event.currentTarget)
       return;
-    this._popupNeedsUpdating = false;
 
-    let viewToolbar = document.getElementById("BMB_viewBookmarksToolbar");
-    if (!this._popupInitialized) {
-      // First popupshowing event, initialize immutable attributes.
-      this._popupInitialized = true;
+    let popup = event.target;
+    let needsUpdate = this._popupNeedsUpdate[popup.id];
+
+    // Check if popup contents need to be updated.  Note that if needsUpdate is
+    // undefined we have never seen the popup, thus it should be updated.
+    if (needsUpdate === false)
+      return;
+    this._popupNeedsUpdate[popup.id] = false;
+
+    function getPlacesAnonymousElement(aAnonId)
+      document.getAnonymousElementByAttribute(popup.parentNode,
+                                              "placesanonid",
+                                              aAnonId);
+
+    let viewToolbarMenuitem = getPlacesAnonymousElement("view-toolbar");
+    if (viewToolbarMenuitem) {
       // Update View bookmarks toolbar checkbox menuitem.
-      viewToolbar.setAttribute("toolbarId", this.personalToolbar.id);
-
-      // Need to set the label on Unsorted Bookmarks menu.
-      let unsortedBookmarksElt =
-        document.getElementById("BMB_unsortedBookmarksFolderMenu");
-      unsortedBookmarksElt.label =
-        PlacesUtils.getString("UnsortedBookmarksFolderTitle");
+      viewToolbarMenuitem.setAttribute("checked",
+                                       !this.personalToolbar.collapsed);
     }
 
-    // Update View Bookmarks Toolbar checkbox menuitem.
-    viewToolbar.setAttribute("checked", !this.personalToolbar.collapsed);
-
-    // Hide Bookmarks Toolbar menu if the button is next to the bookmarks
-    // toolbar item, show them otherwise.
-    let button = this.button;
-    document.getElementById("BMB_bookmarksToolbarFolderMenu").collapsed =
-      button && button.parentNode == this.bookmarksToolbarItem;
+    let toolbarMenuitem = getPlacesAnonymousElement("toolbar-autohide");
+    if (toolbarMenuitem) {
+      // If bookmarks items are visible, hide Bookmarks Toolbar menu and the
+      // separator after it.
+      toolbarMenuitem.collapsed = toolbarMenuitem.nextSibling.collapsed =
+        isElementVisible(this.bookmarksToolbarItem);
+    }
   },
 
   updatePosition: function BMB_updatePosition() {
-    this._popupNeedsUpdating = true;
+    // Popups will have to be updated when the user customizes the UI, or
+    // changes personal toolbar collapsed status.  Both of those location call
+    // updatePosition(), so this is the only point asking for popup updates.
+    for (let popupId in this._popupNeedsUpdate) {
+      this._popupNeedsUpdate[popupId] = true;
+    }
 
     let button = this.button;
     if (!button)

browser/base/content/browser.xul

@@ -613,7 +613,7 @@
                       key="manBookmarkKb"/>
             <menu id="appmenu_bookmarksMenu"
                   class="split-menuitem-menu">
-              <menupopup id="appmenu_bookmarksMenupopup"
+              <menupopup id="appmenu_bookmarksPopup"
                          placespopup="true"
                          context="placesContext"
                          openInTabs="children"
@@ -621,16 +621,9 @@
                          onclick="BookmarksEventHandler.onClick(event);"
                          onpopupshowing="BookmarksMenuButton.onPopupShowing(event);
                                          if (!this.parentNode._placesView)
-                                           new PlacesMenu(event, 'place:folder=BOOKMARKS_MENU');
-                                         this.appendChild(document.getElementById('appmenu_unsortedBookmarks_seperator'));
-                                         this.appendChild(document.getElementById('appmenu_unsortedBookmarks'));"
+                                           new PlacesMenu(event, 'place:folder=BOOKMARKS_MENU');"
                          tooltip="bhTooltip"
                          popupsinherittooltip="true">
-                <menuseparator id="appmenu_unsortedBookmarks_seperator"/>
-                <menuitem id="appmenu_unsortedBookmarks"
-                          label="&appMenuUnsorted.label;"
-                          oncommand="PlacesCommandHook.showPlacesOrganizer('UnfiledBookmarks');"
-                          class="menuitem-iconic"/>
                 <menuitem id="appmenu_showAllBookmarks"
                           label="&showAllBookmarks.label;"
                           command="Browser:ShowAllBookmarks"
@@ -658,6 +651,25 @@
                              onclick="checkForMiddleClick(this, event);"/>
                 </menu>
                 <menuseparator/>
+                <menu id="appmenu_bookmarksToolbar"
+                      placesanonid="toolbar-autohide"
+                      class="menu-iconic bookmark-item"
+                      label="&personalbarCmd.label;"
+                      container="true">
+                  <menupopup id="appmenu_bookmarksToolbarPopup"
+                             placespopup="true"
+                             context="placesContext"
+                             onpopupshowing="if (!this.parentNode._placesView)
+                                               new PlacesMenu(event, 'place:folder=TOOLBAR');"/>
+                </menu>
+                <menuseparator/>
+                <!-- Bookmarks menu items -->
+                <menuseparator builder="end"
+                               class="hide-if-empty-places-result"/>
+                <menuitem id="appmenu_unsortedBookmarks"
+                          label="&appMenuUnsorted.label;"
+                          oncommand="PlacesCommandHook.showPlacesOrganizer('UnfiledBookmarks');"
+                          class="menuitem-iconic"/>
               </menupopup>
             </menu>
           </hbox>
@@ -1009,6 +1021,23 @@
                                      if (!this.parentNode._placesView)
                                        new PlacesMenu(event, 'place:folder=BOOKMARKS_MENU');"
                      tooltip="bhTooltip" popupsinherittooltip="true">
+            <menuitem id="BMB_viewBookmarksToolbar"
+                      placesanonid="view-toolbar"
+                      toolbarId="PersonalToolbar"
+                      type="checkbox"
+                      oncommand="onViewToolbarCommand(event)"
+                      label="&viewBookmarksToolbar.label;"/>
+            <menuseparator/>
+            <menuitem id="BMB_bookmarksShowAll"
+                      label="&showAllBookmarks.label;"
+                      command="Browser:ShowAllBookmarks"
+                      key="manBookmarkKb"/>
+            <menuseparator/>
+            <menuitem id="BMB_bookmarkThisPage"
+                      class="menuitem-iconic"
+                      label="&bookmarkThisPageCmd.label;"
+                      command="Browser:AddBookmarkAs"
+                      key="addBookmarkAsKb"/>
             <menuitem id="BMB_subscribeToPageMenuitem"
                       label="&subscribeToPageMenuitem.label;"
                       oncommand="return FeedHandler.subscribeToFeed(null, event);"
@@ -1022,47 +1051,26 @@
                          oncommand="return FeedHandler.subscribeToFeed(null, event);"
                          onclick="checkForMiddleClick(this, event);"/>
             </menu>
-            <menuitem id="BMB_bookmarkAllTabs"
-                      label="&addCurPagesCmd.label;"
-                      command="Browser:BookmarkAllTabs"
-                      key="bookmarkAllTabsKb"/>
-            <menuitem id="BMB_bookmarksShowAll"
-                      label="&showAllBookmarks.label;"
-                      command="Browser:ShowAllBookmarks"
-                      key="manBookmarkKb"/>
             <menuseparator/>
-            <menuitem id="BMB_viewBookmarksToolbar"
-                      type="checkbox"
-                      oncommand="onViewToolbarCommand(event)"
-                      label="&viewBookmarksToolbar.label;"/>
-            <menuitem id="BMB_viewBookmarksSidebar"
-                      label="&viewBookmarksSidebar.label;"
-                      type="checkbox"
-                      oncommand="toggleSidebar('viewBookmarksSidebar');"
-                      key="viewBookmarksSidebarKb">
-              <observes element="viewBookmarksSidebar" attribute="checked"/>
-            </menuitem>
-            <menuseparator/>
-            <menu id="BMB_bookmarksToolbarFolderMenu"
+            <menu id="BMB_bookmarksToolbar"
+                  placesanonid="toolbar-autohide"
                   class="menu-iconic bookmark-item"
                   label="&personalbarCmd.label;"
                   container="true">
-              <menupopup id="BMB_bookmarksToolbarFolderPopup"
+              <menupopup id="BMB_bookmarksToolbarPopup"
                          placespopup="true"
                          context="placesContext"
                          onpopupshowing="if (!this.parentNode._placesView)
                                            new PlacesMenu(event, 'place:folder=TOOLBAR');"/>
             </menu>
-            <menu id="BMB_unsortedBookmarksFolderMenu"
-                  class="menu-iconic bookmark-item"
-                  container="true">
-              <menupopup id="BMB_unsortedBookmarksFolderPopup"
-                         placespopup="true"
-                         context="placesContext"
-                         onpopupshowing="if (!this.parentNode._placesView)
-                                           new PlacesMenu(event, 'place:folder=UNFILED_BOOKMARKS');"/>
-            </menu>
             <menuseparator/>
+            <!-- Bookmarks menu items -->
+            <menuseparator builder="end"
+                           class="hide-if-empty-places-result"/>
+            <menuitem id="BMB_unsortedBookmarks"
+                      label="&bookmarksMenuButton.unsorted.label;"
+                      oncommand="PlacesCommandHook.showPlacesOrganizer('UnfiledBookmarks');"
+                      class="menuitem-iconic"/>
           </menupopup>
         </toolbarbutton>
       </toolbaritem>

browser/locales/en-US/chrome/browser/browser.dtd

@@ -149,6 +149,7 @@ can reach it easily. -->
 
 <!ENTITY bookmarksMenuButton.label          "Bookmarks">
 <!ENTITY bookmarksMenuButton.tooltip        "Display your bookmarks">
+<!ENTITY bookmarksMenuButton.unsorted.label "Unsorted Bookmarks">
 <!ENTITY viewBookmarksSidebar.label         "Show in Sidebar">
 <!ENTITY viewBookmarksToolbar.label         "View Bookmarks Toolbar">
 

browser/themes/gnomestripe/browser/browser.css

@@ -473,11 +473,11 @@ menuitem:not([type]) {
 }
 
 #bookmarksToolbarFolderMenu,
-#BMB_bookmarksToolbarFolderMenu {
+#BMB_bookmarksToolbar {
   list-style-image: url("chrome://browser/skin/places/bookmarksToolbar.png");  
 }
 
-#unsortedBookmarksFolderMenu {
+#BMB_unsortedBookmarks {
   list-style-image: url("chrome://browser/skin/places/unsortedBookmarks.png");  
 }
 

browser/themes/pinstripe/browser/browser.css

@@ -281,11 +281,11 @@ toolbarbutton.bookmark-item > menupopup {
 }
 
 #bookmarksToolbarFolderMenu,
-#BMB_bookmarksToolbarFolderMenu {
+#BMB_bookmarksToolbar {
   list-style-image: url("chrome://browser/skin/places/bookmarksToolbar.png");
 }
 
-#BMB_unsortedBookmarksFolderMenu {
+#BMB_unsortedBookmarks {
   list-style-image: url("chrome://browser/skin/places/unfiledBookmarks.png");
 }
 

browser/themes/winstripe/browser/browser.css

@@ -274,20 +274,19 @@
   list-style-image: url("chrome://browser/skin/Privacy-16.png");
 }
 
-#appmenu_unsortedBookmarks {
-  list-style-image: url("chrome://browser/skin/places/unsortedBookmarks.png");
-}
-
 #appmenu_addons {
   list-style-image: url("chrome://mozapps/skin/extensions/extensionGeneric-16.png");
 }
 
+#BMB_subscribeToPage:not([disabled]),
+#BMB_subscribeToPageMenu,
 #appmenu_subscribeToPage:not([disabled]),
 #appmenu_subscribeToPageMenu {
   list-style-image: url("chrome://browser/skin/feeds/feed-icons-16.png");
   -moz-image-region: rect(0px 16px 16px 0px);
 }
 
+#BMB_bookmarkThisPage,
 #appmenu_bookmarkThisPage {
   list-style-image: url("chrome://browser/skin/places/bookmark.png");
   -moz-image-region: rect(0 16px 16px 0);
@@ -2001,12 +2000,14 @@ toolbarbutton.bookmark-item[dragover="true"][open="true"] {
 
 /* Bookmarks roots menu-items */
 #bookmarksToolbarFolderMenu,
-#BMB_bookmarksToolbarFolderMenu {
+#appmenu_bookmarksToolbar,
+#BMB_bookmarksToolbar {
   list-style-image: url("chrome://browser/skin/places/bookmarksToolbar.png");
   -moz-image-region: auto;
 }
 
-#BMB_unsortedBookmarksFolderMenu {
+#appmenu_unsortedBookmarks,
+#BMB_unsortedBookmarks {
   list-style-image: url("chrome://browser/skin/places/unsortedBookmarks.png");
   -moz-image-region: auto;
 }