Bug 741506 - Removing nodes while a bookmarks popup is open in OS X native menubar zombifies them.
authorMarco Bonardo <mbonardo@mozilla.com>
Sat, 14 Apr 2012 14:08:43 +0200
changeset 95000 dfa2bf38f209bca2db35bca81452713e716a6109
parent 94976 6880c195054ffdb9ae83e5ae2637540a00c3f01c
child 95001 99f3fe5ff9c4470572d08d9f1653e595c8da6afc
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs741506
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 741506 - Removing nodes while a bookmarks popup is open in OS X native menubar zombifies them. r=Mano
browser/components/places/content/browserPlacesViews.js
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -161,24 +161,36 @@ PlacesViewBase.prototype = {
   },
 
   destroyContextMenu: function PVB_destroyContextMenu(aPopup) {
     this._contextMenuShown = false;
     if (window.content)
       window.content.focus();
   },
 
-  _cleanPopup: function PVB_cleanPopup(aPopup) {
+  _cleanPopup: function PVB_cleanPopup(aPopup, aDelay) {
     // Remove Places nodes from the popup.
     let child = aPopup._startMarker;
     while (child.nextSibling != aPopup._endMarker) {
-      if (child.nextSibling._placesNode)
-        aPopup.removeChild(child.nextSibling);
-      else
+      let sibling = child.nextSibling;
+      if (sibling._placesNode && !aDelay) {
+        aPopup.removeChild(sibling);
+      }
+      else if (sibling._placesNode && aDelay) {
+        // HACK (bug 733419): the popups originating from the OS X native
+        // menubar don't live-update while open, thus we don't clean it
+        // until the next popupshowing, to avoid zombie menuitems.
+        if (!aPopup._delayedRemovals)
+          aPopup._delayedRemovals = [];
+        aPopup._delayedRemovals.push(sibling);
         child = child.nextSibling;
+      }
+      else {
+        child = child.nextSibling;
+      }
     }
   },
 
   _rebuildPopup: function PVB__rebuildPopup(aPopup) {
     let resultNode = aPopup._placesNode;
     if (!resultNode.containerOpen)
       return;
 
@@ -390,17 +402,18 @@ PlacesViewBase.prototype = {
       let stringId = aStatus == Ci.mozILivemark.STATUS_LOADING ?
                        "bookmarksLivemarkLoading" : "bookmarksLivemarkFailed";
       statusMenuitem.setAttribute("label", PlacesUIUtils.getString(stringId));
       if (aPopup._startMarker.nextSibling != statusMenuitem)
         aPopup.insertBefore(statusMenuitem, aPopup._startMarker.nextSibling);
     }
     else {
       // The livemark has finished loading.
-      aPopup.removeChild(aPopup._statusMenuitem);
+      if (aPopup._statusMenuitem.parentNode == aPopup)
+        aPopup.removeChild(aPopup._statusMenuitem);
     }
   },
 
   toggleCutNode: function PVB_toggleCutNode(aNode, aValue) {
     let elt = aNode._DOMElement;
     if (elt) {
       // We may get the popup for menus, but we need the menu itself.
       if (elt.localName == "menupopup")
@@ -666,26 +679,30 @@ PlacesViewBase.prototype = {
         );
       }
     }
   },
 
   _populateLivemarkPopup: function PVB__populateLivemarkPopup(aPopup)
   {
     this._setLivemarkSiteURIMenuItem(aPopup);
-    this._setLivemarkStatusMenuItem(aPopup, Ci.mozILivemark.STATUS_LOADING);
+    // Show the loading status only if there are no entries yet.
+    if (aPopup._startMarker.nextSibling == aPopup._endMarker)
+      this._setLivemarkStatusMenuItem(aPopup, Ci.mozILivemark.STATUS_LOADING);
 
     PlacesUtils.livemarks.getLivemark({ id: aPopup._placesNode.itemId },
       (function (aStatus, aLivemark) {
         let placesNode = aPopup._placesNode;
         if (!Components.isSuccessCode(aStatus) || !placesNode.containerOpen)
           return;
 
-        this._setLivemarkStatusMenuItem(aPopup, aLivemark.status);
-        this._cleanPopup(aPopup);
+        if (aLivemark.status != Ci.mozILivemark.STATUS_LOADING)
+          this._setLivemarkStatusMenuItem(aPopup, aLivemark.status);
+        this._cleanPopup(aPopup,
+          this._nativeView && aPopup.parentNode.hasAttribute("open"));
 
         let children = aLivemark.getNodesForContainer(placesNode);
         for (let i = 0; i < children.length; i++) {
           let child = children[i];
           this.nodeInserted(placesNode, child, i);
           if (child.accessCount)
             child._DOMElement.setAttribute("visited", true);
           else
@@ -830,16 +847,23 @@ PlacesViewBase.prototype = {
   },
 
   _onPopupShowing: function PVB__onPopupShowing(aEvent) {
     // Avoid handling popupshowing of inner views.
     let popup = aEvent.originalTarget;
 
     this._ensureMarkers(popup);
 
+    // Remove any delayed element, see _cleanPopup for details.
+    if ("_delayedRemovals" in popup) {
+      while (popup._delayedRemovals.length > 0) {
+        popup.removeChild(popup._delayedRemovals.shift());
+      }
+    }
+
     if (popup._placesNode && PlacesUIUtils.getViewForNode(popup) == this) {
       if (!popup._placesNode.containerOpen)
         popup._placesNode.containerOpen = true;
       if (!popup._built)
         this._rebuildPopup(popup);
 
       this._mayAddCommandsItems(popup);
     }
@@ -1733,18 +1757,22 @@ PlacesToolbar.prototype = {
 function PlacesMenu(aPopupShowingEvent, aPlace) {
   this._rootElt = aPopupShowingEvent.target; // <menupopup>
   this._viewElt = this._rootElt.parentNode;   // <menu>
   this._viewElt._placesView = this;
   this._addEventListeners(this._rootElt, ["popupshowing", "popuphidden"], true);
   this._addEventListeners(window, ["unload"], false);
 
 #ifdef XP_MACOSX
-  if (this._viewElt.parentNode.localName == "menubar") {
-    this._nativeView = true;
+  // Must walk up to support views in sub-menus, like Bookmarks Toolbar menu.
+  for (let elt = this._viewElt.parentNode; elt; elt = elt.parentNode) {
+    if (elt.localName == "menubar") {
+      this._nativeView = true;
+      break;
+    }
   }
 #endif
 
   PlacesViewBase.call(this, aPlace);
   this._onPopupShowing(aPopupShowingEvent);
 }
 
 PlacesMenu.prototype = {