Bug 1369339 - make sure to flush layout when necessary for the description height workaround in panelviews. r=Paolo
authorMike de Boer <mdeboer@mozilla.com>
Wed, 07 Jun 2017 12:42:05 +0200
changeset 413272 2f14337def2d952661fd523c175c27775f7c000a
parent 413271 1766ee2089e809650c6f44f91c38fa953b335347
child 413273 162d1c8539dccd359502a8abcb2f08cbe98b5bd3
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersPaolo
bugs1369339
milestone55.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 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";