Bug 1199496 - New bookmarks dialog width is increased at each invocation. r=ttaubert

--HG--
extra : commitid : JNuP5Yr3TiT

browser/components/places/PlacesUIUtils.jsm

@@ -461,21 +461,17 @@ this.PlacesUIUtils = {
   showBookmarkDialog:
   function PUIU_showBookmarkDialog(aInfo, aParentWindow) {
     // Preserve size attributes differently based on the fact the dialog has
-    // a folder picker or not.  If the picker is visible, the dialog should
-    // be resizable since it may not show enough content for the folders
-    // hierarchy.
+    // a folder picker or not, since it needs more horizontal space than the
+    // other controls.
     let hasFolderPicker = !("hiddenRows" in aInfo) ||
                           aInfo.hiddenRows.indexOf("folderPicker") == -1;
-    // Use a different chrome url, since this allows to persist different sizes,
-    // based on resizability of the dialog.
+    // Use a different chrome url to persist different sizes.
     let dialogURL = hasFolderPicker ?
                     "chrome://browser/content/places/bookmarkProperties2.xul" :
                     "chrome://browser/content/places/bookmarkProperties.xul";
 
-    let features =
-      "centerscreen,chrome,modal,resizable=" + (hasFolderPicker ? "yes" : "no");
-
-    aParentWindow.openDialog(dialogURL, "",  features, aInfo);
+    let features = "centerscreen,chrome,modal,resizable=yes";
+    aParentWindow.openDialog(dialogURL, "", features, aInfo);
     return ("performed" in aInfo && aInfo.performed);
   },
 

browser/components/places/content/bookmarkProperties.js

@@ -72,6 +72,8 @@ const LIVEMARK_CONTAINER = 2;
 const ACTION_EDIT = 0;
 const ACTION_ADD = 1;
 
+let elementsHeight = new Map();
+
 var BookmarkPropertiesPanel = {
 
   /** UI Text Strings */
@@ -271,6 +273,43 @@ var BookmarkPropertiesPanel = {
     var acceptButton = document.documentElement.getButton("accept");
     acceptButton.label = this._getAcceptLabel();
 
+    // Do not use sizeToContent, otherwise, due to bug 90276, the dialog will
+    // grow at every opening.
+    // Since elements can be uncollapsed asynchronously, we must observe their
+    // mutations and resize the dialog using a cached element size.
+    this._height = window.outerHeight;
+    this._mutationObserver = new MutationObserver(mutations => {
+      for (let mutation of mutations) {
+        let target = mutation.target;
+        let id = target.id;
+        if (!/^editBMPanel_.*(Row|Checkbox)$/.test(id))
+          continue;
+
+        let collapsed = target.getAttribute("collapsed") === "true";
+        let wasCollapsed = mutation.oldValue === "true";
+        if (collapsed == wasCollapsed)
+          continue;
+
+        if (collapsed) {
+          this._height -= elementsHeight.get(id);
+          elementsHeight.delete(id);
+        } else {
+          elementsHeight.set(id, target.boxObject.height);
+          this._height += elementsHeight.get(id);
+        }
+        window.resizeTo(window.outerWidth, this._height);
+      }
+    });
+
+    this._mutationObserver.observe(document,
+                                   { subtree: true,
+                                     attributeOldValue: true,
+                                     attributeFilter: ["collapsed"] });
+
+    // Some controls are flexible and we want to update their cached size when
+    // the dialog is resized.
+    window.addEventListener("resize", this);
+
     this._beginBatch();
 
     switch (this._action) {
@@ -300,25 +339,6 @@ var BookmarkPropertiesPanel = {
         break;
     }
 
-    // Adjust the dialog size to the changes done by initPanel. This is necessary because
-    // initPanel, which shows and hides elements, may run after some async work was done
-    // here - i.e. after the DOM load event was processed.
-    window.sizeToContent();
-
-    // When collapsible elements change their collapsed attribute we must
-    // resize the dialog.
-    // sizeToContent is not usable due to bug 90276, so we'll use resizeTo
-    // instead and cache the element size. See WSucks in the legacy
-    // UI code (addBookmark2.js).
-    if (!this._element("tagsRow").collapsed) {
-      this._element("tagsSelectorRow")
-          .addEventListener("DOMAttrModified", this, false);
-    }
-    if (!this._element("folderRow").collapsed) {
-      this._element("folderTreeRow")
-          .addEventListener("DOMAttrModified", this, false);
-    }
-
     if (!gEditItemOverlay.readOnly) {
       // Listen on uri fields to enable accept button if input is valid
       if (this._itemType == BOOKMARK_ITEM) {
@@ -330,12 +350,9 @@ var BookmarkPropertiesPanel = {
         }
       }
     }
-
-    window.sizeToContent();
   }),
 
   // nsIDOMEventListener
-  _elementsHeight: [],
   handleEvent: function BPP_handleEvent(aEvent) {
     var target = aEvent.target;
     switch (aEvent.type) {
@@ -347,24 +364,11 @@ var BookmarkPropertiesPanel = {
                   .getButton("accept").disabled = !this._inputIsValid();
         }
         break;
-
-      case "DOMAttrModified":
-        // this is called when collapsing a node, but also its direct children,
-        // we only need to resize when the original node changes.
-        if ((target.id == "editBMPanel_tagsSelectorRow" ||
-             target.id == "editBMPanel_folderTreeRow") &&
-            aEvent.attrName == "collapsed" &&
-            target == aEvent.originalTarget) {
-          var id = target.id;
-          var newHeight = window.outerHeight;
-          if (aEvent.newValue) // is collapsed
-            newHeight -= this._elementsHeight[id];
-          else {
-            this._elementsHeight[id] = target.boxObject.height;
-            newHeight += this._elementsHeight[id];
-          }
-
-          window.resizeTo(window.outerWidth, newHeight);
+      case "resize":
+        for (let [id, oldHeight] of elementsHeight) {
+          let newHeight = document.getElementById(id).boxObject.height;
+          this._height += - oldHeight + newHeight;
+          elementsHeight.set(id, newHeight);
         }
         break;
     }
@@ -418,12 +422,13 @@ var BookmarkPropertiesPanel = {
 
   onDialogUnload() {
     // gEditItemOverlay does not exist anymore here, so don't rely on it.
+    this._mutationObserver.disconnect();
+    delete this._mutationObserver;
+
+    window.removeEventListener("resize", this);
+
     // Calling removeEventListener with arguments which do not identify any
     // currently registered EventListener on the EventTarget has no effect.
-    this._element("tagsSelectorRow")
-        .removeEventListener("DOMAttrModified", this, false);
-    this._element("folderTreeRow")
-        .removeEventListener("DOMAttrModified", this, false);
     this._element("locationField")
         .removeEventListener("input", this, false);
   },

browser/components/places/content/editBookmarkOverlay.xul

@@ -24,7 +24,9 @@
         <column flex="1" id="editBMPanel_editColumn" />
       </columns>
       <rows id="editBMPanel_rows">
-        <row align="center" id="editBMPanel_nameRow">
+        <row id="editBMPanel_nameRow"
+             align="center"
+             collapsed="true">
           <label value="&editBookmarkOverlay.name.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.name.accesskey;"
@@ -33,7 +35,9 @@
                    onchange="gEditItemOverlay.onNamePickerChange();"/>
         </row>
 
-        <row align="center" id="editBMPanel_locationRow">
+        <row id="editBMPanel_locationRow"
+             align="center"
+             collapsed="true">
           <label value="&editBookmarkOverlay.location.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.location.accesskey;"
@@ -43,7 +47,9 @@
                    onchange="gEditItemOverlay.onLocationFieldChange();"/>
         </row>
 
-        <row align="center" id="editBMPanel_folderRow">
+        <row id="editBMPanel_folderRow"
+             align="center"
+             collapsed="true">
           <label value="&editBookmarkOverlay.folder.label;"
                  class="editBMPanel_rowLabel"
                  control="editBMPanel_folderMenuList"/>
@@ -76,7 +82,9 @@
           </hbox>
         </row>
 
-        <row id="editBMPanel_folderTreeRow" collapsed="true" flex="1">
+        <row id="editBMPanel_folderTreeRow"
+             collapsed="true"
+             flex="1">
           <spacer/>
           <vbox flex="1">
             <tree id="editBMPanel_folderTree"
@@ -103,7 +111,9 @@
           </vbox>
         </row>
 
-        <row align="center" id="editBMPanel_tagsRow">
+        <row id="editBMPanel_tagsRow"
+             align="center"
+             collapsed="true">
           <label value="&editBookmarkOverlay.tags.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.tags.accesskey;"
@@ -136,7 +146,9 @@
                    height="150"/>
         </row>
 
-        <row align="center" id="editBMPanel_keywordRow">
+        <row id="editBMPanel_keywordRow"
+             align="center"
+             collapsed="true">
           <observes element="additionalInfoBroadcaster" attribute="hidden"/>
           <label value="&editBookmarkOverlay.keyword.label;"
                  class="editBMPanel_rowLabel"
@@ -146,7 +158,8 @@
                    onchange="gEditItemOverlay.onKeywordFieldChange();"/>
         </row>
 
-        <row id="editBMPanel_descriptionRow">
+        <row id="editBMPanel_descriptionRow"
+             collapsed="true">
           <observes element="additionalInfoBroadcaster" attribute="hidden"/>
           <label value="&editBookmarkOverlay.description.label;"
                  class="editBMPanel_rowLabel"
@@ -161,6 +174,7 @@
     </grid>
 
     <checkbox id="editBMPanel_loadInSidebarCheckbox"
+              collapsed="true"
               label="&editBookmarkOverlay.loadInSidebar.label;"
               accesskey="&editBookmarkOverlay.loadInSidebar.accesskey;"
               oncommand="gEditItemOverlay.onLoadInSidebarCheckboxCommand();">