Bug 1501207 - Don't say that an item was both set and not set to grow; r=jdescottes

Differential Revision: https://phabricator.services.mozilla.com/D9504

--HG--
extra : moz-landing-system : lando

devtools/client/inspector/flexbox/components/FlexItemSizingProperties.js

@@ -122,8 +122,9 @@ class FlexItemSizingProperties extends PureComponent {
         [getStr("flexbox.itemSizing.itemBaseSizeFromContent")]);
     }
 
+    const className = "section base";
     return (
-      dom.li({ className: property ? "section" : "section no-property" },
+      dom.li({ className: className + (property ? "" : " no-property") },
         dom.span({ className: "name" },
           getStr("flexbox.itemSizing.baseSizeSectionHeader")
         ),
@@ -150,7 +151,7 @@ class FlexItemSizingProperties extends PureComponent {
     }
 
     const flexGrow = properties["flex-grow"];
-    const flexGrow0 = parseFloat(flexGrow) === 0;
+    const nonZeroFlexGrowDefined = flexGrow && parseFloat(flexGrow) !== 0;
     const flexShrink = properties["flex-shrink"];
     const flexShrink0 = parseFloat(flexShrink) === 0;
     const grew = mainDeltaSize > 0;
@@ -170,13 +171,13 @@ class FlexItemSizingProperties extends PureComponent {
     }
 
     // Then tell users whether the item was set to grow, shrink or none of them.
-    if (flexGrow && !flexGrow0 && lineGrowthState !== "shrinking") {
+    if (nonZeroFlexGrowDefined && lineGrowthState !== "shrinking") {
       reasons.push(getStr("flexbox.itemSizing.setToGrow"));
     }
     if (flexShrink && !flexShrink0 && lineGrowthState !== "growing") {
       reasons.push(getStr("flexbox.itemSizing.setToShrink"));
     }
-    if (!grew && !shrank && lineGrowthState === "growing") {
+    if (!nonZeroFlexGrowDefined && !grew && !shrank && lineGrowthState === "growing") {
       reasons.push(getStr("flexbox.itemSizing.notSetToGrow"));
     }
     if (!grew && !shrank && lineGrowthState === "shrinking") {
@@ -211,7 +212,7 @@ class FlexItemSizingProperties extends PureComponent {
         // because it was later min-clamped.
         reasons.push(getStr("flexbox.itemSizing.shrinkAttemptWhenClamped"));
       }
-    } else if (lineGrowthState === "growing" && flexGrow && !flexGrow0) {
+    } else if (lineGrowthState === "growing" && nonZeroFlexGrowDefined) {
       // The item did not grow or shrink. There was room on the line and flex-grow was
       // set, other items have likely used up all of the space.
       property = this.renderCssProperty("flex-grow", flexGrow);
@@ -244,8 +245,9 @@ class FlexItemSizingProperties extends PureComponent {
       return null;
     }
 
+    const className = "section flexibility";
     return (
-      dom.li({ className: property ? "section" : "section no-property" },
+      dom.li({ className: className + (property ? "" : " no-property") },
         dom.span({ className: "name" },
           getStr("flexbox.itemSizing.flexibilitySectionHeader")
         ),
@@ -271,7 +273,7 @@ class FlexItemSizingProperties extends PureComponent {
     }
 
     return (
-      dom.li({ className: "section" },
+      dom.li({ className: "section min" },
         dom.span({ className: "name" },
           getStr("flexbox.itemSizing.minSizeSectionHeader")
         ),
@@ -293,7 +295,7 @@ class FlexItemSizingProperties extends PureComponent {
     const maxDimensionValue = properties[`max-${dimension}`];
 
     return (
-      dom.li({ className: "section" },
+      dom.li({ className: "section max" },
         dom.span({ className: "name" },
           getStr("flexbox.itemSizing.maxSizeSectionHeader")
         ),
@@ -307,7 +309,7 @@ class FlexItemSizingProperties extends PureComponent {
 
   renderFinalSizeSection({ mainFinalSize }) {
     return (
-      dom.li({ className: "section no-property" },
+      dom.li({ className: "section final no-property" },
         dom.span({ className: "name" },
           getStr("flexbox.itemSizing.finalSizeSectionHeader")
         ),

devtools/client/inspector/flexbox/test/browser.ini

@@ -2,8 +2,9 @@
 tags = devtools
 subsuite = devtools
 support-files =
-  doc_flexbox_simple.html
   doc_flexbox_pseudos.html
+  doc_flexbox_simple.html
+  doc_flexbox_specific_cases.html
   doc_flexbox_text_nodes.html
   head.js
   !/devtools/client/inspector/test/head.js
@@ -20,6 +21,7 @@ support-files =
 [browser_flexbox_item_outline_hidden_when_useless.js]
 [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]

devtools/client/inspector/flexbox/test/browser_flexbox_sizing_grow_and_not_grow.js

@@ -0,0 +1,33 @@
+/* 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");
+
+// Non-regression for bug 1501207.
+// In this bug, an item that was set to grow, with a flex-basis larger than its max-width
+// was marked both as "set to grow" and "not set to grow" at the same time.
+// So this test checks that this does not happen anymore.
+
+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 onFlexibilityReasonsRendered = waitForDOM(doc,
+    "ul.flex-item-sizing .section.flexibility .reasons");
+  await selectNode("#grow-not-grow div:first-child", inspector);
+  const [reasonsList] = await onFlexibilityReasonsRendered;
+
+  info("Get the list of reasons in the flexibility section");
+  const [...reasons] = reasonsList.querySelectorAll("li");
+  ok(reasons.some(r => r.textContent === getStr("flexbox.itemSizing.setToGrow")),
+     "The 'set to grow' reason was found");
+  ok(reasons.every(r => r.textContent !== getStr("flexbox.itemSizing.notSetToGrow")),
+     "The 'not set to grow' reason was not found");
+});

devtools/client/inspector/flexbox/test/doc_flexbox_specific_cases.html

@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<style>
+#grow-not-grow {
+  display:flex;
+  width:500px;
+  border:1px solid;
+}
+#grow-not-grow div:first-child {
+  flex-grow:1;
+  flex-basis:100px;
+  max-width:50px;
+}
+#grow-not-grow div:last-child {
+  flex-grow: 1;
+}
+</style>
+<div id="grow-not-grow">
+  <div>item 1</div>
+  <div>item 2</div>
+</div>