Bug 1551569 - New inactive CSS rule to show warning when margin or padding is used on internal table elements. r=nchevobbe,flod
authorSebastian Zartner <sebastianzartner@gmail.com>
Mon, 11 Jan 2021 07:30:02 +0000
changeset 562603 4d2a6935eb92501f75ca9cb5ace6121793a4dcdc
parent 562602 410530def0b511f090003b6a9e5f7a0d75678007
child 562604 28b8ca785fbb9ce28a87be19010d5e9bba5634a5
push id38094
push userbtara@mozilla.com
push dateMon, 11 Jan 2021 21:51:43 +0000
treeherdermozilla-central@c6d819bd39da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe, flod
bugs1551569
milestone86.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 1551569 - New inactive CSS rule to show warning when margin or padding is used on internal table elements. r=nchevobbe,flod Differential Revision: https://phabricator.services.mozilla.com/D101091
devtools/client/locales/en-US/tooltips.ftl
devtools/server/actors/utils/inactive-property-helper.js
devtools/server/tests/chrome/inactive-property-helper/margin-padding.js
devtools/server/tests/chrome/test_inspector-inactive-property-helper.html
--- a/devtools/client/locales/en-US/tooltips.ftl
+++ b/devtools/client/locales/en-US/tooltips.ftl
@@ -36,16 +36,20 @@ inactive-css-not-display-block-on-floate
 inactive-css-property-is-impossible-to-override-in-visited = It’s impossible to override <strong>{ $property }</strong> due to <strong>:visited</strong> restriction.
 
 inactive-css-position-property-on-unpositioned-box = <strong>{ $property }</strong> has no effect on this element since it’s not a positioned element.
 
 inactive-text-overflow-when-no-overflow = <strong>{ $property }</strong> has no effect on this element since <strong>overflow:hidden</strong> is not set.
 
 inactive-outline-radius-when-outline-style-auto-or-none = <strong>{ $property }</strong> has no effect on this element because its <strong>outline-style</strong> is <strong>auto</strong> or <strong>none</strong>.
 
+inactive-css-not-for-internal-table-elements = <strong>{ $property }</strong> has no effect on internal table elements.
+
+inactive-css-not-for-internal-table-elements-except-table-cells = <strong>{ $property }</strong> has no effect on internal table elements except table cells.
+
 ## 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.
 
 inactive-css-not-grid-or-flex-container-fix = Try adding <strong>display:grid</strong> or <strong>display:flex</strong>. { learn-more }
 
 inactive-css-not-grid-or-flex-container-or-multicol-container-fix = Try adding either <strong>display:grid</strong>, <strong>display:flex</strong>, or <strong>columns:2</strong>. { learn-more }
 
@@ -66,16 +70,20 @@ inactive-css-non-replaced-inline-or-tabl
 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 }
 
 inactive-css-position-property-on-unpositioned-box-fix = Try setting its <strong>position</strong> property to something other than <strong>static</strong>. { learn-more }
 
 inactive-text-overflow-when-no-overflow-fix = Try adding <strong>overflow:hidden</strong>. { learn-more }
 
+inactive-css-not-for-internal-table-elements-fix = Try setting its <strong>display</strong> property to something else than <strong>table-cell</strong>, <strong>table-column</strong>, <strong>table-row</strong>, <strong>table-column-group</strong>, <strong>table-row-group</strong>, or <strong>table-footer-group</strong>. { learn-more }
+
+inactive-css-not-for-internal-table-elements-except-table-cells-fix = Try setting its <strong>display</strong> property to something else than <strong>table-column</strong>, <strong>table-row</strong>, <strong>table-column-group</strong>, <strong>table-row-group</strong>, or <strong>table-footer-group</strong>. { learn-more }
+
 inactive-outline-radius-when-outline-style-auto-or-none-fix = Try setting its <strong>outline-style</strong> property to something other than <strong>auto</strong> or <strong>none</strong>. { learn-more }
 
 ## In the Rule View when a CSS property may have compatibility issues with other browsers
 ## we display an icon. When this icon is hovered this message is displayed to explain why
 ## the property is incompatible and the platforms it is incompatible on.
 ## Variables:
 ##   $property (string) - A CSS declaration name e.g. "-moz-user-select" that can be a platform specific alias.
 ##   $rootProperty (string) - A raw CSS property name e.g. "user-select" that is not a platform specific alias.
--- a/devtools/server/actors/utils/inactive-property-helper.js
+++ b/devtools/server/actors/utils/inactive-property-helper.js
@@ -308,16 +308,60 @@ class InactivePropertyHelper {
           "-moz-outline-radius-bottomleft",
           "-moz-outline-radius-bottomright",
         ],
         when: () => this.checkComputedStyle("outline-style", ["auto", "none"]),
         fixId: "inactive-outline-radius-when-outline-style-auto-or-none-fix",
         msgId: "inactive-outline-radius-when-outline-style-auto-or-none",
         numFixProps: 1,
       },
+      // margin properties used on table internal elements.
+      {
+        invalidProperties: [
+          "margin",
+          "margin-block",
+          "margin-block-end",
+          "margin-block-start",
+          "margin-bottom",
+          "margin-inline",
+          "margin-inline-end",
+          "margin-inline-start",
+          "margin-left",
+          "margin-right",
+          "margin-top",
+        ],
+        when: () => this.internalTableElement,
+        fixId: "inactive-css-not-for-internal-table-elements-fix",
+        msgId: "inactive-css-not-for-internal-table-elements",
+        numFixProps: 1,
+      },
+      // padding properties used on table internal elements except table cells.
+      {
+        invalidProperties: [
+          "padding",
+          "padding-block",
+          "padding-block-end",
+          "padding-block-start",
+          "padding-bottom",
+          "padding-inline",
+          "padding-inline-end",
+          "padding-inline-start",
+          "padding-left",
+          "padding-right",
+          "padding-top",
+        ],
+        when: () =>
+          this.internalTableElement &&
+          !this.checkComputedStyle("display", ["table-cell"]),
+        fixId:
+          "inactive-css-not-for-internal-table-elements-except-table-cells-fix",
+        msgId:
+          "inactive-css-not-for-internal-table-elements-except-table-cells",
+        numFixProps: 1,
+      },
     ];
   }
 
   /**
    * Get a list of unique CSS property names for which there are checks
    * for used/unused state.
    *
    * @return {Set}
@@ -573,33 +617,48 @@ class InactivePropertyHelper {
   /**
    * Check if the current node is a table column.
    */
   get tableColumn() {
     return this.style && this.style.display === "table-column";
   }
 
   /**
-   * Check if the curent node is a horizontal table track. That is: either a table row
+   * Check if the current node is an internal table element.
+   */
+  get internalTableElement() {
+    return this.checkComputedStyle("display", [
+      "table-cell",
+      "table-row",
+      "table-row-group",
+      "table-header-group",
+      "table-footer-group",
+      "table-column",
+      "table-column-group",
+    ]);
+  }
+
+  /**
+   * Check if the current node is a horizontal table track. That is: either a table row
    * displayed in horizontal writing mode, or a table column displayed in vertical writing
    * mode.
    */
   get horizontalTableTrack() {
     if (!this.tableRow && !this.tableColumn) {
       return false;
     }
 
     const wm = this.getTableTrackParentWritingMode();
     const isVertical = wm.includes("vertical") || wm.includes("sideways");
 
     return isVertical ? this.tableColumn : this.tableRow;
   }
 
   /**
-   * Check if the curent node is a vertical table track. That is: either a table row
+   * Check if the current node is a vertical table track. That is: either a table row
    * displayed in vertical writing mode, or a table column displayed in horizontal writing
    * mode.
    */
   get verticalTableTrack() {
     if (!this.tableRow && !this.tableColumn) {
       return false;
     }
 
@@ -619,17 +678,17 @@ class InactivePropertyHelper {
   /**
    * Check if the current node is a table column group.
    */
   get columnGroup() {
     return this.isColumnGroup(this.node);
   }
 
   /**
-   * Check if the curent node is a horizontal table track group. That is: either a table
+   * Check if the current node is a horizontal table track group. That is: either a table
    * row group displayed in horizontal writing mode, or a table column group displayed in
    * vertical writing mode.
    */
   get horizontalTableTrackGroup() {
     if (!this.rowGroup && !this.columnGroup) {
       return false;
     }
 
@@ -638,17 +697,17 @@ class InactivePropertyHelper {
 
     const isHorizontalRowGroup = this.rowGroup && !isVertical;
     const isHorizontalColumnGroup = this.columnGroup && isVertical;
 
     return isHorizontalRowGroup || isHorizontalColumnGroup;
   }
 
   /**
-   * Check if the curent node is a vertical table track group. That is: either a table row
+   * Check if the current node is a vertical table track group. That is: either a table row
    * group displayed in vertical writing mode, or a table column group displayed in
    * horizontal writing mode.
    */
   get verticalTableTrackGroup() {
     if (!this.rowGroup && !this.columnGroup) {
       return false;
     }
 
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/chrome/inactive-property-helper/margin-padding.js
@@ -0,0 +1,260 @@
+/* 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 `align-content` test cases.
+
+export default [
+  {
+    info: "margin is active on block containers",
+    property: "margin",
+    tagName: "div",
+    rules: ["div { margin: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "margin is active on flex containers",
+    property: "margin",
+    tagName: "div",
+    rules: ["div { display: flex; margin: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "margin is active on grid containers",
+    property: "margin",
+    tagName: "div",
+    rules: ["div { display: grid; margin: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "margin is active on tables",
+    property: "margin",
+    tagName: "div",
+    rules: ["div { display: table; margin: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "margin is active on inline tables",
+    property: "margin",
+    tagName: "div",
+    rules: ["div { display: inline-table; margin: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "margin is active on table captions",
+    property: "margin",
+    tagName: "div",
+    rules: ["div { display: table-caption; margin: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "margin is inactive on table cells",
+    property: "margin",
+    tagName: "div",
+    rules: ["div { display: table-cell; margin: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-block is inactive on table cells",
+    property: "margin-block",
+    tagName: "div",
+    rules: ["div { display: table-cell; margin-block: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-block-start is inactive on table cells",
+    property: "margin-block-start",
+    tagName: "div",
+    rules: ["div { display: table-cell; margin-block-start: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-block-end is inactive on table cells",
+    property: "margin-block-end",
+    tagName: "div",
+    rules: ["div { display: table-cell; margin-block-end: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-block is inactive on table cells",
+    property: "margin-block",
+    tagName: "div",
+    rules: ["div { display: table-cell; margin-block: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-bottom is inactive on table rows",
+    property: "margin-bottom",
+    tagName: "div",
+    rules: ["div { display: table-row; margin-bottom: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-inline-start is inactive on table rows",
+    property: "margin-inline-start",
+    tagName: "div",
+    rules: ["div { display: table-row; margin-inline-start: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-inline-end is inactive on table rows",
+    property: "margin-inline-end",
+    tagName: "div",
+    rules: ["div { display: table-row; margin-inline-end: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-inline is inactive on table rows",
+    property: "margin-inline",
+    tagName: "div",
+    rules: ["div { display: table-row; margin-inline: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-left is inactive on table columns",
+    property: "margin-left",
+    tagName: "div",
+    rules: ["div { display: table-column; margin-left: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-right is inactive on table row groups",
+    property: "margin-right",
+    tagName: "div",
+    rules: ["div { display: table-row-group; margin-right: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "margin-top is inactive on table column groups",
+    property: "margin-top",
+    tagName: "div",
+    rules: ["div { display: table-column-group; margin-top: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "padding is active on block containers",
+    property: "padding",
+    tagName: "div",
+    rules: ["div { padding: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding is active on flex containers",
+    property: "padding",
+    tagName: "div",
+    rules: ["div { display: flex; padding: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding is active on grid containers",
+    property: "padding",
+    tagName: "div",
+    rules: ["div { display: grid; padding: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding is active on tables",
+    property: "padding",
+    tagName: "div",
+    rules: ["div { display: table; padding: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding is active on inline tables",
+    property: "padding",
+    tagName: "div",
+    rules: ["div { display: inline-table; padding: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding is active on table captions",
+    property: "padding",
+    tagName: "div",
+    rules: ["div { display: table-caption; padding: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding is active on table cells",
+    property: "padding",
+    tagName: "div",
+    rules: ["div { display: table-cell; padding: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding-block is active on table cells",
+    property: "padding-block",
+    tagName: "div",
+    rules: ["div { display: table-cell; padding-block: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding-block-start is active on table cells",
+    property: "padding-block-start",
+    tagName: "div",
+    rules: ["div { display: table-cell; padding-block-start: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding-block-end is active on table cells",
+    property: "padding-block-end",
+    tagName: "div",
+    rules: ["div { display: table-cell; padding-block-end: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding-block is active on table cells",
+    property: "padding-block",
+    tagName: "div",
+    rules: ["div { display: table-cell; padding-block: 10px; }"],
+    isActive: true,
+  },
+  {
+    info: "padding-bottom is inactive on table rows",
+    property: "padding-bottom",
+    tagName: "div",
+    rules: ["div { display: table-row; padding-bottom: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "padding-inline-start is inactive on table rows",
+    property: "padding-inline-start",
+    tagName: "div",
+    rules: ["div { display: table-row; padding-inline-start: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "padding-inline-end is inactive on table rows",
+    property: "padding-inline-end",
+    tagName: "div",
+    rules: ["div { display: table-row; padding-inline-end: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "padding-inline is inactive on table rows",
+    property: "padding-inline",
+    tagName: "div",
+    rules: ["div { display: table-row; padding-inline: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "padding-left is inactive on table columns",
+    property: "padding-left",
+    tagName: "div",
+    rules: ["div { display: table-column; padding-left: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "padding-right is inactive on table row groups",
+    property: "padding-right",
+    tagName: "div",
+    rules: ["div { display: table-row-group; padding-right: 10px; }"],
+    isActive: false,
+  },
+  {
+    info: "padding-top is inactive on table column groups",
+    property: "padding-top",
+    tagName: "div",
+    rules: ["div { display: table-column-group; padding-top: 10px; }"],
+    isActive: false,
+  },
+];
--- a/devtools/server/tests/chrome/test_inspector-inactive-property-helper.html
+++ b/devtools/server/tests/chrome/test_inspector-inactive-property-helper.html
@@ -42,16 +42,17 @@ SimpleTest.waitForExplicitFinish();
   //                                  of the one that should be tested in isPropertyUsed.
   // - {Boolean} isActive: should the property be active (isPropertyUsed `used` result).
   const testFiles = [
     "align-content.js",
     "align-place-self.js",
     "float.js",
     "gap.js",
     "grid-with-absolute-properties.js",
+    "margin-padding.js",
     "max-min-width-height.js",
     "place-items-content.js",
     "positioned.js",
     "vertical-align.js",
     "text-overflow.js",
     "outline-radius.js",
   ].map(file => `${FOLDER}/${file}`);