Bug 1551581 - New InactiveCSS rule to show warning when display:* used on a floated element; r=miker,pbro,fluent-reviewers,flod

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

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

devtools/client/locales/en-US/tooltips.ftl

@@ -31,6 +31,8 @@ inactive-css-not-inline-or-tablecell = <strong>{ $property }</strong> has no eff
 
 inactive-css-property-because-of-display = <strong>{ $property }</strong> has no effect on this element since it has a display of <strong>{ $display }</strong>.
 
+inactive-css-not-display-block-on-floated = The <strong>display</strong> value has been changed by the engine to <strong>block</strong> because the element is <strong>floated<strong>.
+
 ## In the Rule View when a CSS property cannot be successfully applied we display
 ## an icon. When this icon is hovered this message is displayed to explain how
 ## the problem can be solved.
@@ -54,3 +56,5 @@ inactive-css-not-inline-or-tablecell-fix = Try adding <strong>display:inline</st
 inactive-css-non-replaced-inline-or-table-row-or-row-group-fix = Try adding <strong>display:inline-block</strong> or <strong>display:block</strong>. { learn-more }
 
 inactive-css-non-replaced-inline-or-table-column-or-column-group-fix = Try adding <strong>display:inline-block</strong>. { learn-more }
+
+inactive-css-not-display-block-on-floated-fix = Try removing <strong>float</strong> or adding <strong>display:block</strong>. { learn-more }

devtools/server/actors/utils/inactive-property-helper.js

@@ -172,6 +172,29 @@ class InactivePropertyHelper {
         msgId: "inactive-css-property-because-of-display",
         numFixProps: 1,
       },
+      {
+        invalidProperties: ["display"],
+        when: () =>
+          this.isFloated &&
+          this.checkResolvedStyle("display", [
+            "inline",
+            "inline-block",
+            "inline-table",
+            "inline-flex",
+            "inline-grid",
+            "table-cell",
+            "table-row",
+            "table-row-group",
+            "table-header-group",
+            "table-footer-group",
+            "table-column",
+            "table-column-group",
+            "table-caption",
+          ]),
+        fixId: "inactive-css-not-display-block-on-floated-fix",
+        msgId: "inactive-css-not-display-block-on-floated",
+        numFixProps: 2,
+      },
     ];
   }
 
@@ -334,33 +357,36 @@ class InactivePropertyHelper {
    * @param {Array} values
    *        Values to compare against.
    */
-  checkStyle(propName, values) {
+  checkComputedStyle(propName, values) {
-    return this.checkStyleForNode(this.node, propName, values);
-  }
-
-  /**
-   * Check if a node's propName is set to one of the values passed in the values
-   * array.
-   *
-   * @param {DOMNode} node
-   *        The node to check.
-   * @param {String} propName
-   *        Property name to check.
-   * @param {Array} values
-   *        Values to compare against.
-   */
-  checkStyleForNode(node, propName, values) {
     if (!this.style) {
       return false;
     }
     return values.some(value => this.style[propName] === value);
   }
 
+  /**
+   * Check if a rule's propName is set to one of the values passed in the values
+   * array.
+   *
+   * @param {String} propName
+   *        Property name to check.
+   * @param {Array} values
+   *        Values to compare against.
+   */
+  checkResolvedStyle(propName, values) {
+    if (!(this.cssRule && this.cssRule.style)) {
+      return false;
+    }
+    const { style } = this.cssRule;
+
+    return values.some(value => style[propName] === value);
+  }
+
   /**
    *  Check if the current node is an inline-level box.
    */
   isInlineLevel() {
-    return this.checkStyle("display", [
+    return this.checkComputedStyle("display", [
       "inline",
       "inline-block",
       "inline-table",
@@ -379,7 +405,7 @@ class InactivePropertyHelper {
    * of `display:flex` or `display:inline-flex`.
    */
   get flexContainer() {
-    return this.checkStyle("display", ["flex", "inline-flex"]);
+    return this.checkComputedStyle("display", ["flex", "inline-flex"]);
   }
 
   /**
@@ -394,7 +420,7 @@ class InactivePropertyHelper {
    * of `display:grid` or `display:inline-grid`.
    */
   get gridContainer() {
-    return this.checkStyle("display", ["grid", "inline-grid"]);
+    return this.checkComputedStyle("display", ["grid", "inline-grid"]);
   }
 
   /**
@@ -409,8 +435,8 @@ class InactivePropertyHelper {
    * `column-width` or `column-count` property is not `auto`.
    */
   get multiColContainer() {
-    const autoColumnWidth = this.checkStyle("column-width", ["auto"]);
+    const autoColumnWidth = this.checkComputedStyle("column-width", ["auto"]);
-    const autoColumnCount = this.checkStyle("column-count", ["auto"]);
+    const autoColumnCount = this.checkComputedStyle("column-count", ["auto"]);
 
     return !autoColumnWidth || !autoColumnCount;
   }
@@ -475,6 +501,13 @@ class InactivePropertyHelper {
     return !this.replaced;
   }
 
+  /**
+   * Check if the current node is floated
+   */
+  get isFloated() {
+    return this.style && this.style.cssFloat !== "none";
+  }
+
   /**
    * Check if the current node is a replaced element i.e. an element with
    * content that will be replaced e.g. <img>, <audio>, <video> or <object>

devtools/server/tests/mochitest/inactive-property-helper/float.js

@@ -0,0 +1,51 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// InactivePropertyHelper `float` test cases.
+export default [
+  {
+    info: "display: inline is inactive on a floated element",
+    property: "display",
+    tagName: "div",
+    rules: ["div { display: inline; float: right; }"],
+    isActive: false,
+  },
+  {
+    info: "display: block is active on a floated element",
+    property: "display",
+    tagName: "div",
+    rules: ["div { display: block; float: right;}"],
+    isActive: true,
+  },
+  {
+    info: "display: inline-grid is inactive on a floated element",
+    property: "display",
+    createTestElement: rootNode => {
+      const container = document.createElement("div");
+      container.classList.add("test");
+      rootNode.append(container);
+      return container;
+    },
+    rules: [
+      "div { float: left; display:block; }",
+      ".test { display: inline-grid ;}",
+    ],
+    isActive: false,
+  },
+  {
+    info: "display: table-footer-group is inactive on a floated element",
+    property: "display",
+    createTestElement: rootNode => {
+      const container = document.createElement("div");
+      container.style.display = "table";
+      const footer = document.createElement("div");
+      footer.classList.add("table-footer");
+      container.append(footer);
+      rootNode.append(container);
+      return footer;
+    },
+    rules: [".table-footer { display: table-footer-group; float: left;}"],
+    isActive: false,
+  },
+];

devtools/server/tests/mochitest/test_inspector-inactive-property-helper.html

@@ -47,6 +47,7 @@ SimpleTest.waitForExplicitFinish();
     "max-min-width-height.js",
     "place-items-content.js",
     "vertical-align.js",
+    "float.js"
   ].map(file => `${FOLDER}/${file}`);
 
   // Import all the test cases