Bug 1369339 - make sure to flush layout when necessary for the description height workaround in panelviews. r?paolo draft
authorMike de Boer <mdeboer@mozilla.com>
Wed, 07 Jun 2017 12:42:05 +0200
changeset 590239 8131e356fc62030fe45ce02edd0f70b5d71b534d
parent 590232 48feb087da695afc8f7caa19813a6d1388bc80ed
child 590302 d8f0e3fd45dfd0cd80d7f9df5166c9fd2aacfb16
push id62648
push usermdeboer@mozilla.com
push dateWed, 07 Jun 2017 10:45:38 +0000
reviewerspaolo
bugs1369339
milestone55.0a1
Bug 1369339 - make sure to flush layout when necessary for the description height workaround in panelviews. r?paolo This caused regressions in various panels, like the Identity panel, resulting in cut-off descriptions and labels. Our first concern is correctness, then we try performance. MozReview-Commit-ID: GH7BZ9waXeW
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -1001,32 +1001,37 @@ this.PanelMultiView = class {
     // First we reset any change we may have made previously. The first time
     // this is called, and in the best case scenario, this has no effect.
     let items = [];
     for (let element of viewNode.querySelectorAll(
          "description:not([hidden]):not([value]),toolbarbutton[wrap]:not([hidden])")) {
       // Take the label for toolbarbuttons; it only exists on those elements.
       element = element.labelElement || element;
 
-      let bounds = this._dwu.getBoundsWithoutFlushing(element);
+      let bounds = element.getBoundingClientRect();
       let previous = this._multiLineElementsMap.get(element);
-      // Only remove the 'height' property, which will cause a layout flush, when
-      // absolutely necessary.
+      // We don't need to (re-)apply the workaround for invisible elements or
+      // on elements we've seen before and haven't changed in the meantime.
       if (!bounds.width || !bounds.height ||
           (previous && element.textContent == previous.textContent &&
                        bounds.width == previous.bounds.width)) {
         continue;
       }
 
-      element.style.removeProperty("height");
       items.push({ element });
     }
 
+    // Removing the 'height' property will only cause a layout flush in the next
+    // loop below if it was set.
+    for (let item of items) {
+      item.element.style.removeProperty("height");
+    }
+
     // We now read the computed style to store the height of any element that
-    // may contain wrapping text, which will be zero if the element is hidden.
+    // may contain wrapping text.
     for (let item of items) {
       item.bounds = item.element.getBoundingClientRect();
     }
 
     // Now we can make all the necessary DOM changes at once.
     for (let { element, bounds } of items) {
       this._multiLineElementsMap.set(element, { bounds, textContent: element.textContent });
       element.style.height = bounds.height + "px";