Backed out changeset aea0e7fadf7d (bug 1502002)for ES lint failure on components/FlexItemSizingOutline.js "CLOSED TREE"
authorarthur.iakab <aiakab@mozilla.com>
Fri, 26 Oct 2018 10:11:45 +0300
changeset 443096 2db84dbb7f99acbe326b372d03be97620b27fba0
parent 443095 4579afb8b513704a4c668ebcf774dac31b25ab04
child 443097 8412429e66ea29a7bdd7e3cee7063ff0850d5bef
child 443116 e4759dfa794a9693a49382fe5c96f2d9b2a381be
child 443140 9d294202030e9426769682c2901fe95503868b2b
push id109294
push usernerli@mozilla.com
push dateFri, 26 Oct 2018 09:50:32 +0000
treeherdermozilla-inbound@8412429e66ea [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1502002
milestone65.0a1
backs outaea0e7fadf7db4948b4bf8a818902ed79cbdab7d
first release with
nightly linux32
2db84dbb7f99 / 65.0a1 / 20181026100128 / files
nightly linux64
2db84dbb7f99 / 65.0a1 / 20181026100128 / files
nightly mac
2db84dbb7f99 / 65.0a1 / 20181026100128 / files
nightly win32
2db84dbb7f99 / 65.0a1 / 20181026100128 / files
nightly win64
2db84dbb7f99 / 65.0a1 / 20181026100128 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset aea0e7fadf7d (bug 1502002)for ES lint failure on components/FlexItemSizingOutline.js "CLOSED TREE"
devtools/client/inspector/flexbox/components/FlexItemSizingOutline.js
devtools/client/inspector/flexbox/components/FlexItemSizingProperties.js
devtools/client/inspector/flexbox/test/browser.ini
devtools/client/inspector/flexbox/test/browser_flexbox_sizing_wanted_to_grow_but_was_clamped.js
devtools/client/inspector/flexbox/test/doc_flexbox_specific_cases.html
devtools/server/actors/layout.js
--- a/devtools/client/inspector/flexbox/components/FlexItemSizingOutline.js
+++ b/devtools/client/inspector/flexbox/components/FlexItemSizingOutline.js
@@ -63,38 +63,43 @@ class FlexItemSizingOutline extends Pure
       flexItemSizing,
       properties,
     } = this.props.flexItem;
     const {
       mainBaseSize,
       mainDeltaSize,
       mainMaxSize,
       mainMinSize,
-      clampState,
     } = flexItemSizing;
 
     const isRow = this.props.flexDirection.startsWith("row");
     const dimension = isRow ? "width" : "height";
 
     // Calculate the final size. This is base + delta, then clamped by min or max.
     let mainFinalSize = mainBaseSize + mainDeltaSize;
     mainFinalSize = Math.max(mainFinalSize, mainMinSize);
     mainFinalSize = Math.min(mainFinalSize, mainMaxSize);
 
     // Just don't display anything if there isn't anything useful.
     if (!mainFinalSize && !mainBaseSize && !mainDeltaSize) {
       return null;
     }
 
-    // The max size is only interesting to show if it did clamp the item.
-    const showMax = clampState === "clamped_to_max";
+    // The max size is only interesting to show if it did clamp the item
+    // TODO: replace this with the new clamping state that the API will return once bug
+    // 1498273 is fixed.
+    const showMax = mainMaxSize === mainFinalSize;
 
     // The min size is only really interesting if it actually clamped the item.
-    // TODO: We might also want to check if the min-size property is defined.
-    const showMin = clampState === "clamped_to_min";
+    // Just checking that the main size = final size isn't enough because this may be true
+    // if the max content size is the final size. So also check that min-width/height is
+    // set.
+    // TODO: replace this with the new clamping state that the API will return once bug
+    // 1498273 is fixed.
+    const showMin = mainMinSize === mainFinalSize && properties[`min-${dimension}`];
 
     // Sort all of the dimensions in order to come up with a grid track template.
     // Make mainDeltaSize start from the same point as the other ones so we can compare.
     let sizes = [
       { name: "basis-end", size: mainBaseSize },
       { name: "final-end", size: mainFinalSize },
     ];
 
@@ -143,16 +148,16 @@ class FlexItemSizingOutline extends Pure
           },
           this.renderPoint("basis"),
           this.renderPoint("final"),
           showMin ? this.renderPoint("min") : null,
           showMax ? this.renderPoint("max") : null,
           this.renderBasisOutline(mainBaseSize),
           this.renderDeltaOutline(mainDeltaSize),
           this.renderFinalOutline(mainFinalSize, mainMaxSize, mainMinSize,
-                                  clampState !== "unclamped")
+                                  showMin || showMax)
         )
       )
     );
   }
 }
 
 module.exports = FlexItemSizingOutline;
--- a/devtools/client/inspector/flexbox/components/FlexItemSizingProperties.js
+++ b/devtools/client/inspector/flexbox/components/FlexItemSizingProperties.js
@@ -92,25 +92,30 @@ class FlexItemSizingProperties extends P
   renderReasons(sentences) {
     return (
       dom.ul({ className: "reasons" },
         sentences.map(sentence => dom.li({}, sentence))
       )
     );
   }
 
-  renderBaseSizeSection({ mainBaseSize, clampState }, properties, dimension) {
+  renderBaseSizeSection({ mainBaseSize, mainMinSize }, properties, dimension) {
     const flexBasisValue = properties["flex-basis"];
     const dimensionValue = properties[dimension];
     const minDimensionValue = properties[`min-${dimension}`];
+    const hasMinClamping = mainMinSize && mainMinSize === mainBaseSize;
 
     let property = null;
     let reason = null;
 
-    if (flexBasisValue) {
+    if (hasMinClamping && minDimensionValue) {
+      // If min clamping happened, then the base size is going to be that value.
+      // TODO: this isn't going to be necessarily true after bug 1498273 is fixed.
+      property = this.renderCssProperty(`min-${dimension}`, minDimensionValue);
+    } else if (flexBasisValue && !hasMinClamping) {
       // If flex-basis is defined, then that's what is used for the base size.
       property = this.renderCssProperty("flex-basis", flexBasisValue);
     } else if (dimensionValue) {
       // If not and width/height is defined, then that's what defines the base size.
       property = this.renderCssProperty(dimension, dimensionValue);
     } else {
       // Finally, if nothing is set, then the base size is the max-content size.
       reason = this.renderReasons(
@@ -133,31 +138,32 @@ class FlexItemSizingProperties extends P
   }
 
   renderFlexibilitySection(flexItemSizing, properties) {
     const {
       mainDeltaSize,
       mainBaseSize,
       mainFinalSize,
       lineGrowthState,
-      clampState,
     } = flexItemSizing;
 
     // Don't attempt to display anything useful if everything is 0.
     if (!mainFinalSize && !mainBaseSize && !mainDeltaSize) {
       return null;
     }
 
     const flexGrow = properties["flex-grow"];
     const nonZeroFlexGrowDefined = flexGrow && parseFloat(flexGrow) !== 0;
     const flexShrink = properties["flex-shrink"];
     const flexShrink0 = parseFloat(flexShrink) === 0;
     const grew = mainDeltaSize > 0;
     const shrank = mainDeltaSize < 0;
-    const wasClamped = clampState !== "unclamped";
+    // TODO: replace this with the new clamping state that the API will return once bug
+    // 1498273 is fixed.
+    const wasClamped = mainDeltaSize + mainBaseSize !== mainFinalSize;
 
     const reasons = [];
 
     // First output a sentence for telling users about whether there was enough room or
     // not on the line.
     if (lineGrowthState === "growing") {
       reasons.push(getStr("flexbox.itemSizing.extraRoomOnLine"));
     } else if (lineGrowthState === "shrinking") {
@@ -251,40 +257,45 @@ class FlexItemSizingProperties extends P
           this.getFlexibilityValueString(grew, mainDeltaSize)
         ),
         property,
         this.renderReasons(reasons)
       )
     );
   }
 
-  renderMinimumSizeSection({ clampState, mainMinSize }, properties, dimension) {
+  renderMinimumSizeSection({ mainMinSize, mainFinalSize }, properties, dimension) {
     // We only display the minimum size when the item actually violates that size during
     // layout & is clamped.
-    if (clampState !== "clamped_to_min") {
+    // For now, we detect this by checking that the min-size is the same as the final size
+    // and that a min-size is actually defined in CSS.
+    // TODO: replace this with the new clamping state that the API will return once bug
+    // 1498273 is fixed.
+    const minDimensionValue = properties[`min-${dimension}`];
+    if (mainMinSize !== mainFinalSize || !minDimensionValue) {
       return null;
     }
 
-    const minDimensionValue = properties[`min-${dimension}`];
-
     return (
       dom.li({ className: "section min" },
         dom.span({ className: "name" },
           getStr("flexbox.itemSizing.minSizeSectionHeader")
         ),
         dom.span({ className: "value theme-fg-color1" },
           this.getRoundedDimension(mainMinSize)
         ),
         this.renderCssProperty(`min-${dimension}`, minDimensionValue)
       )
     );
   }
 
-  renderMaximumSizeSection({ clampState, mainMaxSize }, properties, dimension) {
-    if (clampState !== "clamped_to_max") {
+  renderMaximumSizeSection({ mainMaxSize, mainFinalSize }, properties, dimension) {
+    // TODO: replace this with the new clamping state that the API will return once bug
+    // 1498273 is fixed.
+    if (mainMaxSize !== mainFinalSize) {
       return null;
     }
 
     const maxDimensionValue = properties[`max-${dimension}`];
 
     return (
       dom.li({ className: "section max" },
         dom.span({ className: "name" },
--- a/devtools/client/inspector/flexbox/test/browser.ini
+++ b/devtools/client/inspector/flexbox/test/browser.ini
@@ -22,10 +22,9 @@ support-files =
 [browser_flexbox_item_outline_rotates_for_column.js]
 [browser_flexbox_pseudo_elements_are_listed.js]
 [browser_flexbox_sizing_grow_and_not_grow.js]
 [browser_flexbox_sizing_info_exists.js]
 [browser_flexbox_sizing_info_for_pseudos.js]
 [browser_flexbox_sizing_info_for_text_nodes.js]
 [browser_flexbox_sizing_info_has_correct_sections.js]
 [browser_flexbox_sizing_unrelated_to_siblings_when_clamped.js]
-[browser_flexbox_sizing_wanted_to_grow_but_was_clamped.js]
 [browser_flexbox_text_nodes_are_listed.js]
deleted file mode 100644
--- a/devtools/client/inspector/flexbox/test/browser_flexbox_sizing_wanted_to_grow_but_was_clamped.js
+++ /dev/null
@@ -1,34 +0,0 @@
-/* vim: set ts=2 et sw=2 tw=80: */
-/* Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const { getStr } = require("devtools/client/inspector/layout/utils/l10n");
-
-// Test the specific max-clamping scenario where an item wants to grow a certain amount
-// but its max-size prevents it from growing that much.
-
-const TEST_URI = URL_ROOT + "doc_flexbox_specific_cases.html";
-
-add_task(async function() {
-  await addTab(TEST_URI);
-  const { inspector, flexboxInspector } = await openLayoutView();
-  const { document: doc } = flexboxInspector;
-
-  info("Select the test item in the document and wait for the sizing info to render");
-  const onRendered = waitForDOM(doc, ".flex-outline, .flex-item-sizing", 2);
-  await selectNode("#want-to-grow-more-than-max div", inspector);
-  const [outlineContainer, sizingContainer] = await onRendered;
-
-  info("Check that the outline contains the max point and that it's equal to final");
-  const maxPoint = outlineContainer.querySelector(".flex-outline-point.max");
-  ok(maxPoint, "The max point is displayed");
-  ok(outlineContainer.style.gridTemplateColumns.includes("[final-end max]"),
-     "The final and max points are at the same position");
-
-  info("Check that the flexibility sizing section displays the right info");
-  const reasons = [...sizingContainer.querySelectorAll(".reasons li")];
-  ok(reasons.some(r => r.textContent === getStr("flexbox.itemSizing.growthAttemptWhenClamped")),
-     "The 'wanted to grow but was clamped' reason was found");
-});
--- a/devtools/client/inspector/flexbox/test/doc_flexbox_specific_cases.html
+++ b/devtools/client/inspector/flexbox/test/doc_flexbox_specific_cases.html
@@ -9,25 +9,13 @@
 #grow-not-grow div:first-child {
   flex-grow:1;
   flex-basis:100px;
   max-width:50px;
 }
 #grow-not-grow div:last-child {
   flex-grow: 1;
 }
-
-#want-to-grow-more-than-max {
-  width: 500px;
-  display: flex;
-}
-#want-to-grow-more-than-max div {
-  flex: 1;
-  max-width: 200px;
-}
 </style>
 <div id="grow-not-grow">
   <div>item 1</div>
   <div>item 2</div>
 </div>
-<div id="want-to-grow-more-than-max">
-  <div>item wants to grow more</div>
-</div>
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -106,17 +106,16 @@ const FlexboxActor = ActorClassWithSpec(
         flexItemActors.push(new FlexItemActor(this, item.node, {
           crossMaxSize: item.crossMaxSize,
           crossMinSize: item.crossMinSize,
           mainBaseSize: item.mainBaseSize,
           mainDeltaSize: item.mainDeltaSize,
           mainMaxSize: item.mainMaxSize,
           mainMinSize: item.mainMinSize,
           lineGrowthState: line.growthState,
-          clampState: item.clampState,
         }));
       }
     }
 
     return flexItemActors;
   },
 });