Backed out 1 changesets (bug 1199496) for bc1 test failurs in browser_bookmarksProperties.js
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Mon, 07 Sep 2015 08:54:30 +0200
changeset 289145 441f51977790af03ac4555063c8d97e0600f8e08
parent 289144 28994d94c110726d42dac81e8ede6da16ad5365d
child 289146 a4a86b5a85fd316ace93f4759efa06600e88dbd1
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1199496
milestone42.0a2
backs out7ba5523ffb28d1124978af627d921b9c0a956717
Backed out 1 changesets (bug 1199496) for bc1 test failurs in browser_bookmarksProperties.js Backed out changeset 7ba5523ffb28 (bug 1199496)
browser/components/places/PlacesUIUtils.jsm
browser/components/places/content/bookmarkProperties.js
browser/components/places/content/editBookmarkOverlay.xul
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -462,27 +462,31 @@ this.PlacesUIUtils = {
    *        Owner window for the new dialog.
    *
    * @see documentation at the top of bookmarkProperties.js
    * @return true if any transaction has been performed, false otherwise.
    */
   showBookmarkDialog:
   function PUIU_showBookmarkDialog(aInfo, aParentWindow) {
     // 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.
+    // 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.
     let hasFolderPicker = !("hiddenRows" in aInfo) ||
                           aInfo.hiddenRows.indexOf("folderPicker") == -1;
-    // Use a different chrome url to persist different sizes.
+    // Use a different chrome url, since this allows to persist different sizes,
+    // based on resizability of the dialog.
     let dialogURL = hasFolderPicker ?
                     "chrome://browser/content/places/bookmarkProperties2.xul" :
                     "chrome://browser/content/places/bookmarkProperties.xul";
 
-    let features = "centerscreen,chrome,modal,resizable=yes";
-    aParentWindow.openDialog(dialogURL, "", features, aInfo);
+    let features =
+      "centerscreen,chrome,modal,resizable=" + (hasFolderPicker ? "yes" : "no");
+
+    aParentWindow.openDialog(dialogURL, "",  features, aInfo);
     return ("performed" in aInfo && aInfo.performed);
   },
 
   _getTopBrowserWin: function PUIU__getTopBrowserWin() {
     return RecentWindow.getMostRecentBrowserWindow();
   },
 
   /**
--- a/browser/components/places/content/bookmarkProperties.js
+++ b/browser/components/places/content/bookmarkProperties.js
@@ -67,18 +67,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 
 const BOOKMARK_ITEM = 0;
 const BOOKMARK_FOLDER = 1;
 const LIVEMARK_CONTAINER = 2;
 
 const ACTION_EDIT = 0;
 const ACTION_ADD = 1;
 
-let elementsHeight = new Map();
-
 var BookmarkPropertiesPanel = {
 
   /** UI Text Strings */
   __strings: null,
   get _strings() {
     if (!this.__strings) {
       this.__strings = document.getElementById("stringBundle");
     }
@@ -268,53 +266,16 @@ var BookmarkPropertiesPanel = {
    */
   onDialogLoad: Task.async(function* () {
     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,
-                                   { 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:
         gEditItemOverlay.initPanel({ node: this._node
                                    , hiddenRows: this._hiddenRows });
         acceptButton.disabled = gEditItemOverlay.readOnly;
         break;
@@ -334,46 +295,81 @@ var BookmarkPropertiesPanel = {
 
         // 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;
     }
 
+    // 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) {
         this._element("locationField")
             .addEventListener("input", this, false);
         if (this._isAddKeywordDialog) {
           this._element("keywordField")
               .addEventListener("input", this, false);
         }
       }
     }
+
+    window.sizeToContent();
   }),
 
   // nsIDOMEventListener
+  _elementsHeight: [],
   handleEvent: function BPP_handleEvent(aEvent) {
     var target = aEvent.target;
     switch (aEvent.type) {
       case "input":
         if (target.id == "editBMPanel_locationField" ||
             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);
+
+      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);
         }
         break;
     }
   },
 
 	// Hack for implementing batched-Undo around the editBookmarkOverlay
 	// instant-apply code. For all the details see the comment above beginBatch
 	// in browser-places.js
@@ -417,23 +413,22 @@ var BookmarkPropertiesPanel = {
   },
 
   _element: function BPP__element(aID) {
     return document.getElementById("editBMPanel_" + aID);
   },
 
   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);
   },
 
   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
--- a/browser/components/places/content/editBookmarkOverlay.xul
+++ b/browser/components/places/content/editBookmarkOverlay.xul
@@ -19,42 +19,36 @@
     </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 id="editBMPanel_nameRow"
-             align="center"
-             collapsed="true">
+        <row align="center" id="editBMPanel_nameRow">
           <label value="&editBookmarkOverlay.name.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.name.accesskey;"
                  control="editBMPanel_namePicker"/>
           <textbox id="editBMPanel_namePicker"
                    onchange="gEditItemOverlay.onNamePickerChange();"/>
         </row>
 
-        <row id="editBMPanel_locationRow"
-             align="center"
-             collapsed="true">
+        <row align="center" id="editBMPanel_locationRow">
           <label value="&editBookmarkOverlay.location.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.location.accesskey;"
                  control="editBMPanel_locationField"/>
           <textbox id="editBMPanel_locationField"
                    class="uri-element"
                    onchange="gEditItemOverlay.onLocationFieldChange();"/>
         </row>
 
-        <row id="editBMPanel_folderRow"
-             align="center"
-             collapsed="true">
+        <row align="center" id="editBMPanel_folderRow">
           <label value="&editBookmarkOverlay.folder.label;"
                  class="editBMPanel_rowLabel"
                  control="editBMPanel_folderMenuList"/>
           <hbox flex="1" align="center">
             <menulist id="editBMPanel_folderMenuList"
                       class="folder-icon"
                       flex="1"
                       oncommand="gEditItemOverlay.onFolderMenuListCommand(event);">
@@ -77,19 +71,17 @@
                     class="expander-down"
                     tooltiptext="&editBookmarkOverlay.foldersExpanderDown.tooltip;"
                     tooltiptextdown="&editBookmarkOverlay.foldersExpanderDown.tooltip;"
                     tooltiptextup="&editBookmarkOverlay.expanderUp.tooltip;"
                     oncommand="gEditItemOverlay.toggleFolderTreeVisibility();"/>
           </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"
                   flex="1"
                   class="placesTree"
                   type="places"
                   height="150"
                   minheight="150"
@@ -106,19 +98,17 @@
               <button label="&editBookmarkOverlay.newFolderButton.label;"
                       id="editBMPanel_newFolderButton"
                       accesskey="&editBookmarkOverlay.newFolderButton.accesskey;"
                       oncommand="gEditItemOverlay.newFolder().catch(Components.utils.reportError);"/>
             </hbox>
           </vbox>
         </row>
 
-        <row id="editBMPanel_tagsRow"
-             align="center"
-             collapsed="true">
+        <row align="center" id="editBMPanel_tagsRow">
           <label value="&editBookmarkOverlay.tags.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.tags.accesskey;"
                  control="editBMPanel_tagsField"/>
           <hbox flex="1" align="center">
             <textbox id="editBMPanel_tagsField"
                      type="autocomplete"
                      class="padded"
@@ -141,45 +131,41 @@
         <row id="editBMPanel_tagsSelectorRow"
              align="center"
              collapsed="true">
           <spacer/>
           <listbox id="editBMPanel_tagsSelector"
                    height="150"/>
         </row>
 
-        <row id="editBMPanel_keywordRow"
-             align="center"
-             collapsed="true">
+        <row align="center" id="editBMPanel_keywordRow">
           <observes element="additionalInfoBroadcaster" attribute="hidden"/>
           <label value="&editBookmarkOverlay.keyword.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.keyword.accesskey;"
                  control="editBMPanel_keywordField"/>
           <textbox id="editBMPanel_keywordField"
                    onchange="gEditItemOverlay.onKeywordFieldChange();"/>
         </row>
 
-        <row id="editBMPanel_descriptionRow"
-             collapsed="true">
+        <row id="editBMPanel_descriptionRow">
           <observes element="additionalInfoBroadcaster" attribute="hidden"/>
           <label value="&editBookmarkOverlay.description.label;"
                  class="editBMPanel_rowLabel"
                  accesskey="&editBookmarkOverlay.description.accesskey;"
                  control="editBMPanel_descriptionField"/>
           <textbox id="editBMPanel_descriptionField"
                    multiline="true"
                    rows="4"
                    onchange="gEditItemOverlay.onDescriptionFieldChange();"/>
         </row>
       </rows>
     </grid>
 
     <checkbox id="editBMPanel_loadInSidebarCheckbox"
-              collapsed="true"
               label="&editBookmarkOverlay.loadInSidebar.label;"
               accesskey="&editBookmarkOverlay.loadInSidebar.accesskey;"
               oncommand="gEditItemOverlay.onLoadInSidebarCheckboxCommand();">
       <observes element="additionalInfoBroadcaster" attribute="hidden"/>
     </checkbox>
 
     <!-- If the ids are changing or additional fields are being added, be sure
          to sync the values in places.js -->