[Inactive CSS] Display a warning when min-width, min-height etc. are used incorrectly

VERIFIED FIXED in Firefox 69

Status

enhancement
P2
normal
VERIFIED FIXED
2 months ago
7 days ago

People

(Reporter: miker, Assigned: nchevobbe)

Tracking

(Blocks 1 bug)

unspecified
Firefox 69
Dependency tree / graph

Firefox Tracking Flags

(firefox69 verified)

Details

Attachments

(1 attachment)

Applies to all elements but non-replaced inline elements, table rows, and row groups:

  "max-width",
  "min-width",
  "width",

[This property] doesn't have an effect because it cannot be used on elements other than non-replaced inline elements, table rows, and row groups


Applies to all elements but non-replaced inline elements, table columns, and column groups:

[
  "height",
  "max-height",
  "min-height",
]

[This property] doesn't have an effect because it cannot be used on elements other than non-replaced inline elements, table columns, and column groups

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED

Hello Mike,

I started to look at this and have a few question:

  1. What are considered row groups? <tbody>, <thead>, <tfoot> ?
  2. What are considered column groups? <colgroup> ?
  3. I'm not sure, but I think we can have a fix for non-replaced inline (e.g. use display: inline-block), but I'm not sure the same applies for table elements?
Flags: needinfo?(mratcliffe)

What are considered row groups? <tbody>, <thead>, <tfoot> ?
What are considered column groups? <colgroup> ?

Nope, we can't consider the element names... it all depends on their style.display properties.

I'm not sure, but I think we can have a fix for non-replaced inline (e.g. use display: inline-block), but I'm not sure the same applies for table elements?

"Non-replaced inline elements" is a term used in the spec... just add these helpers:

/**
 * Check if the current node is a table row group.
 */
get tableRowGroup() {
  return this.style.display === "table-row-group";
}

/**
 * Check if the current node is a table column.
 */
get tableColumn() {
  return this.style.display === "table-column";
}

/**
 * Check if the current node is a table column group.
 */
get tableColumnGroup() {
  return this.style.display === "table-column-group";
}

/**
 * Check if the current node is a non-replaced inline box.
 */
get nonReplacedInlineBox() {
  return this.nonReplaced && this.style.display === "inline";
}

/**
 * Check if the current node is a non-replaced element. See `replaced()` for
 * a description of what a replaced element is.
 */
get nonReplaced() {
  return !this.replaced;
}

/**
 * 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>
 * elements.
 */
get replaced() {
  // The <applet> element was removed in Gecko 56 so we can ignore them.
  // These are always treated as replaced elements:
  if (this.nodeNameOneOf([
    "br", "button", "canvas", "embed", "hr", "iframe", "math",
    "object", "picture", "svg", "video",
  ])) {
    return true;
  }

  // audio – Treated as a replaced element only when it's "exposing a user
  // interface element" i.e. has a "controls" attribute.
  if (this.nodeName === "audio" && this.node.getAttribute("controls")) {
    return true;
  }

  // img tags are replaced elements only when the image has finished loading.
  if (this.nodeName === "img" && this.node.complete) {
    return true;
  }

  return false;
}

/**
 * Return the current node's nodeName.
 */
get nodeName() {
  return this.node.nodeName;
}

/**
 * Check if the current node's nodeName matches a value inside the value array.
 *
 * @param {Array} values
 *        Array of values to compare against.
 */
nodeNameOneOf(values) {
  return values.includes(this.nodeName);
}

And use this in your validator:

when: () => !this.nonReplacedInlineBox && !this.tableColumn && !this.tableColumnGroup
Flags: needinfo?(mratcliffe)

This introduce a new form of invalid messages. Until now, all the
messages were like "X has no effect on this since it's not Y".
But in this case, the width/height properties applies to all but
a few cases, which means we can't really keep the same shape of
message (or it would be "since it's not A, B, C, D, ...).
So we're switching to a message that prints the display property
of the element ("X has no effect on this element since it has a
display of Y").
In order to do that, we need to pipe the element computed display
into the inactive tooltip.

Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/31cec3975dd0
Display a warning when min-width, min-height etc. are used incorrectly. r=miker,fluent-reviewers,flod.
Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69

Verified with 69.0b3.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.