Bug 1249536 - Set default height of folder tree correctly and do the resize correctly. r=Ratty a=Ratty
authorFrank-Rainer Grahl <frgrahl@gmx.net>
Thu, 16 Jun 2016 23:05:24 +0200
changeset 31298 675872ac277f815fef26a535bd85a8578b8a4974
parent 31297 60e236584a283661e41dc0f21e21184fb1be8475
child 31299 5084048788caf44c9678226077f9873e77e31850
push id1
push userclokep@gmail.com
push dateMon, 07 May 2018 22:45:56 +0000
treeherdercomm-esr60@57eacde5ef40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersRatty, Ratty
bugs1249536, 1199496
Bug 1249536 - Set default height of folder tree correctly and do the resize correctly. r=Ratty a=Ratty See Bug 1199496.
suite/browser/browser-prefs.js
suite/common/bookmarks/bm-props.js
suite/common/bookmarks/bm-props.xul
suite/common/bookmarks/editBookmarkOverlay.xul
suite/common/jar.mn
suite/modules/PlacesUIUtils.jsm
--- a/suite/browser/browser-prefs.js
+++ b/suite/browser/browser-prefs.js
@@ -219,21 +219,19 @@ pref("places.frecency.unvisitedTypedBonu
 pref("browser.bookmarks.autoExportHTML", false);
 
 // The maximum number of daily bookmark backups to
 // keep in {PROFILEDIR}/bookmarkbackups. Special values:
 // -1: unlimited
 //  0: no backups created (and deletes all existing backups)
 pref("browser.bookmarks.max_backups", 10);
 
-// Don't try to alter these prefs, they'll be reset the next time you use the
+// Don't try to alter this pref. It will be reset the next time you use the
 // bookmarking dialog.
 pref("browser.bookmarks.editDialog.firstEditField", "namePicker");
-pref("browser.bookmarks.editDialog.expandTags", false);
-pref("browser.bookmarks.editDialog.expandFolders", false);
 
 // Tabbed browser
 pref("browser.tabs.loadDivertedInBackground", false);
 pref("browser.tabs.loadInBackground", true);
 pref("browser.tabs.opentabfor.doubleclick", false);
 pref("browser.tabs.opentabfor.middleclick", true);
 pref("browser.tabs.opentabfor.urlbar", true);
 pref("browser.tabs.tooltippreview.enable", true);
--- a/suite/common/bookmarks/bm-props.js
+++ b/suite/common/bookmarks/bm-props.js
@@ -60,16 +60,18 @@
 
 const BOOKMARK_ITEM = 0;
 const BOOKMARK_FOLDER = 1;
 const LIVEMARK_CONTAINER = 2;
 
 const ACTION_EDIT = 0;
 const ACTION_ADD = 1;
 
+var elementsHeight = new Map();
+
 var BookmarkPropertiesPanel = {
 
   /** UI Text Strings */
   __strings: null,
   get _strings() {
     if (!this.__strings) {
       this.__strings = document.getElementById("stringBundle");
     }
@@ -248,17 +250,16 @@ var BookmarkPropertiesPanel = {
                                .then(aLivemark => {
             this._itemType = LIVEMARK_CONTAINER;
             this._feedURI = aLivemark.feedURI;
             this._siteURI = aLivemark.siteURI;
             this._fillEditProperties();
 
             document.documentElement
                     .getButton("accept").disabled = !this._inputIsValid();
-            window.outerHeight += this._element("nameRow").boxObject.height * 2;
           }, () => undefined);
       }
 
       // Description
       if (PlacesUtils.annotations
                      .itemHasAnnotation(this._itemId, PlacesUIUtils.DESCRIPTION_ANNO)) {
         this._description = PlacesUtils.annotations
                                        .getItemAnnotation(this._itemId,
@@ -291,53 +292,70 @@ var BookmarkPropertiesPanel = {
    */
   onDialogLoad: function BPP_onDialogLoad() {
     this._determineItemInfo();
 
     document.title = this._getDialogTitle();
     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,
+                                   { attributes: true,
+                                     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) {
       case ACTION_EDIT:
         this._fillEditProperties();
         acceptButton.disabled = this._readOnly;
         break;
       case ACTION_ADD:
         this._createNewItem().then(() => this._fillAddProperties());
         // if this is an uri related dialog disable accept button until
         // the user fills an uri value.
         if (this._itemType == BOOKMARK_ITEM)
           acceptButton.disabled = !this._inputIsValid();
         break;
     }
 
-    // When collapsible elements change their collapsed attribute we must
-    // resize the dialog.
-    var observer = new MutationObserver(function(aRecords, aObserver) {
-      var el = document.getElementById("editBookmarkPanelContent");
-      var width = el.boxObject.width;
-      window.sizeToContent();
-      window.outerWidth -= el.boxObject.width - width;
-    });
-    if (!this._element("tagsRow").collapsed) {
-      if (Services.prefs.getBoolPref("browser.bookmarks.editDialog.expandTags"))
-        gEditItemOverlay.toggleTagsSelector();
-      observer.observe(this._element("tagsSelectorRow"),
-                       {attributes: true, attributeFilter: ["collapsed"]});
-    }
-    if (!this._element("folderRow").collapsed) {
-      if (Services.prefs.getBoolPref("browser.bookmarks.editDialog.expandFolders"))
-        gEditItemOverlay.toggleFolderTreeVisibility();
-      observer.observe(this._element("folderTreeRow"),
-                       {attributes: true, attributeFilter: ["collapsed"]});
-    }
-
     if (!this._readOnly) {
       // Listen on uri fields to enable accept button if input is valid
       if (this._itemType == BOOKMARK_ITEM) {
         this._element("locationField")
             .addEventListener("input", this, false);
         if (this._isAddKeywordDialog) {
           this._element("keywordField")
               .addEventListener("input", this, false);
@@ -345,43 +363,40 @@ var BookmarkPropertiesPanel = {
       }
       else if (this._itemType == LIVEMARK_CONTAINER) {
         this._element("feedLocationField")
             .addEventListener("input", this, false);
         this._element("siteLocationField")
             .addEventListener("input", this, false);
       }
     }
-
-    window.sizeToContent();
-    window.addEventListener("resize", this, false);
   },
 
   // nsIDOMEventListener
   handleEvent: function BPP_handleEvent(aEvent) {
     var target = aEvent.target;
     switch (aEvent.type) {
       case "input":
         if (target.id == "editBMPanel_locationField" ||
             target.id == "editBMPanel_feedLocationField" ||
             target.id == "editBMPanel_siteLocationField" ||
             target.id == "editBMPanel_keywordField") {
           // Check uri fields to enable accept button if input is valid
           document.documentElement
                   .getButton("accept").disabled = !this._inputIsValid();
         }
         break;
+      case "resize":
+        for (let [id, oldHeight] of elementsHeight) {
+          let newHeight = document.getElementById(id).boxObject.height;
+          this._height += - oldHeight + newHeight;
+          elementsHeight.set(id, newHeight);
+        break;
+    }
 
-      case "resize":
-        ["folderTree", "tagsSelector", "description"].forEach(function(e) {
-          var el = document.getElementById("editBMPanel_" + e + "Row");
-          if (el.boxObject.height)
-            el.height = el.boxObject.height;
-        });
-        break;
     }
   },
 
   _beginBatch: function BPP__beginBatch() {
     if (this._batching)
       return;
 
     PlacesUtils.transactionManager.beginBatch(null);
@@ -407,52 +422,47 @@ var BookmarkPropertiesPanel = {
     gEditItemOverlay.initPanel(this._itemId,
                                { hiddenRows: this._hiddenRows });
     // Empty location field if the uri is about:blank, this way inserting a new
     // url will be easier for the user, Accept button will be automatically
     // disabled by the input listener until the user fills the field.
     var locationField = this._element("locationField");
     if (locationField.value == "about:blank")
       locationField.value = "";
-    if (this._itemType == BOOKMARK_ITEM)
-      acceptButton.disabled = !this._inputIsValid();
-    window.sizeToContent();
   },
 
   // nsISupports
   QueryInterface: function BPP_QueryInterface(aIID) {
     if (aIID.equals(Components.interfaces.nsIDOMEventListener) ||
         aIID.equals(Components.interfaces.nsISupports))
       return this;
 
     throw Components.results.NS_NOINTERFACE;
   },
 
   _element: function BPP__element(aID) {
     return document.getElementById("editBMPanel_" + aID);
   },
 
   onDialogUnload: function BPP_onDialogUnload() {
+
+  // gEditItemOverlay does not exist anymore here, so don't rely on it.
+    this._mutationObserver.disconnect();
+    delete this._mutationObserver;
+
     window.removeEventListener("resize", this, false);
-    // gEditItemOverlay does not exist anymore here, so don't rely on it.
     // Calling removeEventListener with arguments which do not identify any
     // currently registered EventListener on the EventTarget has no effect.
     this._element("locationField")
         .removeEventListener("input", this, false);
     this._element("feedLocationField")
         .removeEventListener("input", this, false);
     this._element("siteLocationField")
         .removeEventListener("input", this, false);
 
-    if (!this._element("tagsRow").collapsed)
-      Services.prefs.setBoolPref("browser.bookmarks.editDialog.expandTags",
-                                 !this._element("tagsSelectorRow").collapsed);
-    if (!this._element("folderRow").collapsed)
-      Services.prefs.setBoolPref("browser.bookmarks.editDialog.expandFolders",
-                                 !this._element("folderTreeRow").collapsed);
   },
 
   onDialogAccept: function BPP_onDialogAccept() {
     // We must blur current focused element to save its changes correctly
     document.commandDispatcher.focusedElement.blur();
     // The order here is important! We have to uninit the panel first, otherwise
     // late changes could force it to commit more transactions.
     gEditItemOverlay.uninitPanel(false);
@@ -571,17 +581,17 @@ var BookmarkPropertiesPanel = {
     for (var i = 0; i < this._URIs.length; ++i) {
       var uri = this._URIs[i];
       var title = this._titles[i] || this._getURITitleFromHistory(uri);
       var createTxn = new PlacesCreateBookmarkTransaction(uri, -1,
                                                           PlacesUtils.bookmarks.DEFAULT_INDEX,
                                                           title);
       transactions.push(createTxn);
     }
-    return transactions; 
+    return transactions;
   },
 
   /**
    * Returns a transaction for creating a new folder item representing the
    * various fields and opening arguments of the dialog.
    */
   _getCreateNewFolderTransaction:
   function BPP__getCreateNewFolderTransaction(aContainer, aIndex) {
@@ -617,17 +627,17 @@ var BookmarkPropertiesPanel = {
     var txn;
 
     switch (this._itemType) {
       case BOOKMARK_FOLDER:
         txn = this._getCreateNewFolderTransaction(container, index);
         break;
       case LIVEMARK_CONTAINER:
         txn = this._getCreateNewLivemarkTransaction(container, index);
-        break;      
+        break;
       default: // BOOKMARK_ITEM
         txn = this._getCreateNewBookmarkTransaction(container, index);
     }
 
     PlacesUtils.transactionManager.doTransaction(txn);
     var promise = txn._promise || {
       then: function(callback) {
         callback();
--- a/suite/common/bookmarks/bm-props.xul
+++ b/suite/common/bookmarks/bm-props.xul
@@ -21,23 +21,23 @@
         buttons="accept, cancel"
         buttoniconaccept="save"
         ondialogaccept="BookmarkPropertiesPanel.onDialogAccept();"
         ondialogcancel="BookmarkPropertiesPanel.onDialogCancel();"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         onload="BookmarkPropertiesPanel.onDialogLoad();"
         onunload="BookmarkPropertiesPanel.onDialogUnload();"
         style="min-width: 30em;"
-        persist="screenX screenY width">
+        persist="screenX screenY height width">
 
   <stringbundleset id="stringbundleset">
     <stringbundle id="stringBundle"
                   src="chrome://communicator/locale/bookmarks/bm-props.properties"/>
   </stringbundleset>
 
   <script type="application/javascript"
           src="chrome://communicator/content/bookmarks/editBookmarkOverlay.js"/>
   <script type="application/javascript"
           src="chrome://communicator/content/bookmarks/bm-props.js"/>
 
-<vbox id="editBookmarkPanelContent" flex="1"/>
+<vbox id="editBookmarkPanelContent"/>
 
 </dialog>
--- a/suite/common/bookmarks/editBookmarkOverlay.xul
+++ b/suite/common/bookmarks/editBookmarkOverlay.xul
@@ -23,62 +23,72 @@
     </hbox>
 
     <grid id="editBookmarkPanelGrid" flex="1">
       <columns id="editBMPanel_columns">
         <column id="editBMPanel_labelColumn" />
         <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;"
                  control="editBMPanel_namePicker"
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_namePicker"
                    onblur="gEditItemOverlay.onNamePickerChange();"
                    observes="paneElementsBroadcaster"/>
         </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;"
                  control="editBMPanel_locationField"
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_locationField"
                    class="uri-element"
                    onblur="gEditItemOverlay.onLocationFieldBlur();"
                    observes="paneElementsBroadcaster"/>
         </row>
 
-        <row align="center" id="editBMPanel_feedLocationRow">
+        <row id="editBMPanel_feedLocationRow"
+             align="center"
+             collapsed="true">
           <label value="&editBookmarkOverlay.feedLocation.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.feedLocation.accesskey;"
                  control="editBMPanel_feedLocationField"
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_feedLocationField"
                    class="uri-element"
                    observes="paneElementsBroadcaster"/>
         </row>
 
-        <row align="center" id="editBMPanel_siteLocationRow">
+        <row id="editBMPanel_siteLocationRow"
+             align="center"
+             collapsed="true">
           <label value="&editBookmarkOverlay.siteLocation.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.siteLocation.accesskey;"
                  control="editBMPanel_siteLocationField"
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_siteLocationField"
                    class="uri-element"
                    observes="paneElementsBroadcaster"/>
         </row>
 
-        <row align="center" id="editBMPanel_folderRow">
+        <row id="editBMPanel_folderRow"
+             align="center"
+             collapsed="true">
           <label value="&editBookmarkOverlay.folder.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.folder.accesskey;"
                  control="editBMPanel_folderMenuList"
                  observes="paneElementsBroadcaster"/>
           <hbox flex="1" align="center">
             <menulist id="editBMPanel_folderMenuList"
                       class="folder-icon"
@@ -107,25 +117,24 @@
                     tooltiptextup="&editBookmarkOverlay.expanderUp.tooltip;"
                     oncommand="gEditItemOverlay.toggleFolderTreeVisibility();"
                     observes="paneElementsBroadcaster"/>
           </hbox>
         </row>
 
         <row id="editBMPanel_folderTreeRow"
              collapsed="true"
-             persist="height"
-             flex="2">
+             flex="1">
           <spacer/>
           <vbox flex="1">
             <tree id="editBMPanel_folderTree"
                   class="placesTree"
                   type="places"
                   treelines="true"
-                  height="150"
+                  minheight="150"
                   flex="1"
                   editable="true"
                   onselect="gEditItemOverlay.onFolderTreeSelect();"
                   hidecolumnpicker="true"
                   observes="paneElementsBroadcaster">
               <treecols>
                 <treecol anonid="title" flex="1" primary="true" hideheader="true"/>
               </treecols>
@@ -137,28 +146,30 @@
                       id="editBMPanel_newFolderButton"
                       accesskey="&editBookmarkOverlay.newFolderButton.accesskey;"
                       tooltiptext="&editBookmarkOverlay.newFolderButton.tooltip;"
                       oncommand="gEditItemOverlay.newFolder();"/>
             </hbox>
           </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;"
                  control="editBMPanel_tagsField"
                  observes="paneElementsBroadcaster"/>
           <hbox flex="1" align="center">
             <textbox id="editBMPanel_tagsField"
                      type="autocomplete"
                      class="padded"
                      flex="1"
-                     autocompletesearch="places-tag-autocomplete" 
+                     autocompletesearch="places-tag-autocomplete"
                      completedefaultindex="true"
                      tabscrolling="true"
                      showcommentcolumn="true"
                      onblur="gEditItemOverlay.onTagsFieldBlur();"
                      observes="paneElementsBroadcaster"
                      placeholder="&editBookmarkOverlay.tagsEmptyDesc.label;"/>
             <button id="editBMPanel_tagsSelectorExpander"
                     class="expander-down"
@@ -166,39 +177,40 @@
                     tooltiptextdown="&editBookmarkOverlay.tagsExpanderDown.tooltip;"
                     tooltiptextup="&editBookmarkOverlay.expanderUp.tooltip;"
                     oncommand="gEditItemOverlay.toggleTagsSelector();"
                     observes="paneElementsBroadcaster"/>
           </hbox>
         </row>
 
         <row id="editBMPanel_tagsSelectorRow"
-             persist="height"
-             collapsed="true"
-             flex="2">
+             align="center"
+             collapsed="true">
           <spacer/>
           <listbox id="editBMPanel_tagsSelector"
-                   height="150"
-                   flex="1"
+                   minheight="150"
                    observes="paneElementsBroadcaster"/>
         </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"
                  accesskey="&editBookmarkOverlay.keyword.accesskey;"
                  control="editBMPanel_keywordField"
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_keywordField"
                    onblur="gEditItemOverlay.onKeywordFieldBlur();"
                    observes="paneElementsBroadcaster"/>
         </row>
 
-        <row id="editBMPanel_descriptionRow" persist="height" flex="1">
+        <row id="editBMPanel_descriptionRow"
+             collapsed="true">
           <observes element="additionalInfoBroadcaster" attribute="hidden"/>
           <label value="&editBookmarkOverlay.description.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.description.accesskey;"
                  control="editBMPanel_descriptionField"
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_descriptionField"
                    multiline="true"
--- a/suite/common/jar.mn
+++ b/suite/common/jar.mn
@@ -111,16 +111,18 @@ comm.jar:
    content/communicator/bindings/general.xml                        (bindings/general.xml)
    content/communicator/bindings/notification.xml                   (bindings/notification.xml)
    content/communicator/bindings/toolbar.xml                        (bindings/toolbar.xml)
 *  content/communicator/bindings/prefwindow.xml                     (bindings/prefwindow.xml)
    content/communicator/bookmarks/bm-panel.js                       (bookmarks/bm-panel.js)
    content/communicator/bookmarks/bm-panel.xul                      (bookmarks/bm-panel.xul)
    content/communicator/bookmarks/bm-props.js                       (bookmarks/bm-props.js)
    content/communicator/bookmarks/bm-props.xul                      (bookmarks/bm-props.xul)
+# Provide another URI for the bookmark properties dialog so we can persist the attributes separately
+% override chrome://communicator/content/bookmarks/bm-props2.xul chrome://communicator/content/bookmarks/bm-props.xul
    content/communicator/bookmarks/bookmarksManager.css              (bookmarks/bookmarksManager.css)
    content/communicator/bookmarks/bookmarksManager.js               (bookmarks/bookmarksManager.js)
    content/communicator/bookmarks/bookmarksManager.xul              (bookmarks/bookmarksManager.xul)
    content/communicator/bookmarks/browser-places.js                 (bookmarks/browser-places.js)
    content/communicator/bookmarks/editBookmarkOverlay.js            (bookmarks/editBookmarkOverlay.js)
    content/communicator/bookmarks/editBookmarkOverlay.xul           (bookmarks/editBookmarkOverlay.xul)
    content/communicator/bookmarks/moveBookmarks.js                  (bookmarks/moveBookmarks.js)
    content/communicator/bookmarks/moveBookmarks.xul                 (bookmarks/moveBookmarks.xul)
--- a/suite/modules/PlacesUIUtils.jsm
+++ b/suite/modules/PlacesUIUtils.jsm
@@ -632,18 +632,27 @@ var PlacesUIUtils = {
    *        See documentation at the top of bm-props.js
    * @param aMinimalUI
    *        [optional] if true, the dialog is opened by its alternative
    *        chrome: uri.
    *
    * @return true if any transaction has been performed, false otherwise.
    */
   _showBookmarkDialog: function PUIU__showBookmarkDialog(aInfo) {
-    var dialogURL = "chrome://communicator/content/bookmarks/bm-props.xul";
-    var features = "centerscreen,chrome,modal,resizable=yes";
+    // Preserve size attributes differently based on the fact the dialog has
+    // a folder picker or not, since it needs more horizontal space than the
+    // other controls.
+    let hasFolderPicker = !("hiddenRows" in aInfo) ||
+                          !aInfo.hiddenRows.includes("folderPicker");
+    // Use a different chrome url to persist different sizes.
+    let dialogURL = hasFolderPicker ?
+                    "chrome://communicator/content/bookmarks/bm-props2.xul" :
+                    "chrome://communicator/content/bookmarks/bm-props.xul";
+
+    let features = "centerscreen,chrome,modal,resizable=yes";
     this._getCurrentActiveWin().openDialog(dialogURL, "", features, aInfo);
     return ("performed" in aInfo && aInfo.performed);
   },
 
   _getTopBrowserWin: function PUIU__getTopBrowserWin() {
     return this._getCurrentActiveWin().gPrivate ||
            Services.wm.getMostRecentWindow("navigator:browser");
   },