848539 - Remove support for "min-width: auto" and "min-height: auto", since they're being dropped from flexbox spec

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Per this thread:
 http://lists.w3.org/Archives/Public/www-style/2013Mar/0137.html
it appears that "min-width: auto" and "min-height: auto" are being removed from the flexbox spec. This requires a CSSWG resolution (which hasn't happened yet), but for the moment I'll assume that a resolution will pass, and I'm filing this bug to track what we need to do when it does.

To deal with this change, we'll need to do the following (at least):

 (a) back out bug 763689, effectively (modulo bitrot)

 (b) Ideally, provide an alternate way for authors to request min-height:min-content on flex items, so that they can use flex-shrink in vertical flex containers without clipping content. (We don't support "-moz-min-content" at all for height or min-height right now -- perhaps we should add support for it, and restrict it to flex containers? e.g. make "min-height:-moz-min-content" behave like "min-height:auto" did before...?)

 (c) Fix all of our flexbox unit tests that assume min-size:auto is the default value to either change their expected rendering or (better) explicitly specify "min-[width|height]: -moz-min-content" to preserve their existing rendering

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> This requires a CSSWG resolution (which hasn't
> happened yet), but for the moment I'll assume that a resolution will pass

Looks like this resolution passed:

> # [17:29] <fantasai> glazou: Any objections to the proposal?
> # [17:29] <fantasai> RESOLVED: Reset min-width/min-height to zero for Flex Items
Reference: http://krijnhoetmer.nl/irc-logs/css/20130313#l-332

Comment 2

[Daniel Holbert [:dholbert]]

(and the ED of the spec has been updated: https://dvcs.w3.org/hg/csswg/rev/9437131b3d6e )

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
>  (b) Ideally, provide an alternate way for authors to request
> min-height:min-content on flex items

I think we're going to punt on this part -- I'd rather wait for "real" min-content height support, instead of introducing an only-working-in-flexbox version of it (or introducing a hacky vendor-specific keyword as a stopgap).  I filed bug 852367 as a tracking bug for this, so that I've got that bug # to reference in a few needing-to-be-disabled reftests.

I'll be posting patches shortly for this (covering the rest of comment 0) -- basically backing out all of the min-[width|height]:auto chunks that landed on various bugs, in reverse chronological order.  The patches will be as follows:
{
* part 1: Back out bug 794748 (mochitest for min-width:auto).

* part 2: Back out bug 666041 part 8 (special-handling for min-width:auto as min-content, for flex items).

* part 3: Back out part of bug 666041 part 7 (flexbox frame class impl -- just removing the code that deals with min-height:auto).

* part 4: Back out bug 763689 part 3 (style-system handling for min-height:auto).

* part 5: Back out bug 763689 part 2 (style-system handling for min-width:auto).

* part 6: Back out bug 763689 part 1 (CSS parser support for min-width:auto & min-height:auto).

* part 7: Fix or disable reftests that depend on min-width:auto / min-height:auto.
}

Each patch (aside from the final reftest-fixup one) was generated by doing:
  hg up -r $CSETID_TO_BACKOUT
  hg revert -r $PARENT_OF_THAT_CSET
  hg qnew backout-$CSETID_TO_BACKOUT
This generates a clean backout patch.  After building a patch-stack of these, I qpushed them in order (backing out the newest stuff first) and fixed bitrot as-necessary.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 1: Back out bug 794748 (mochitest for min-width:auto)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: part 2: Back out bug 666041 part 8 (special-handling for min-width:auto as min-content, for flex items)

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #5)
> Created attachment 727473 [details] [diff] [review]
> part 2: Back out bug 666041 part 8 (special-handling for min-width:auto as
> min-content, for flex items)

(One note on this part -- this intentionally leaves behind some XXXdholbert comments that mention min-width:auto. Those will get removed in part 5.  I'm doing it like that so as to keep these patches as direct backouts, aside from bitrot-fixes.)

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: part 3: Back out part of bug 666041 part 7 (flexbox frame class impl -- just removing the code that deals with min-height:auto)

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: part 4: Back out bug 763689 part 3 (style-system handling for min-height:auto).

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: part 5: Back out bug 763689 part 2 (style-system handling for min-width:auto).

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: part 6: Back out bug 763689 part 1 (CSS parser support for min-width:auto & min-height:auto)

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: part 7: Fix or disable reftests that depend on min-width:auto / min-height:auto.

This reftest-patch:
 - Fixes comments in reftests to say "min-content" instead of "auto" (and indicate that this is no longer the default behavior)

 - Adds "min-width: -moz-min-content" to all the tests that were previously exercising our "min-width: auto" behavior.

 - Adds "min-height: -moz-min-content" (with a comment indicating that it's currently non-functional) to all the tests that were previously exercising our "min-height: auto" behavior.  (Note: most of these tests still pass, because they're testing that large "height" values don't influence our min-height, when we have min-height:auto/min-content.  Currently, it'll trivially not influence our min-height, because our min-height will be 0.)

- removes one no-longer-necessary manual "min-width:0" specification (which was there to override the formerly-default "min-width:auto")

Comment 12

[Daniel Holbert [:dholbert]]

For reference, I've pasted the original csets that are being backed out, below each patch-title, below:

> * part 1: Back out bug 794748 (mochitest for min-width:auto).
https://hg.mozilla.org/mozilla-central/rev/ed5120ffd118

> * part 2: Back out bug 666041 part 8 (special-handling for min-width:auto as
> min-content, for flex items).
https://hg.mozilla.org/mozilla-central/rev/de6a5c46a8ff

> * part 3: Back out part of bug 666041 part 7 (flexbox frame class impl --
> just removing the code that deals with min-height:auto).
https://hg.mozilla.org/mozilla-central/rev/076d87bf30d0

> * part 4: Back out bug 763689 part 3 (style-system handling for
> min-height:auto).
https://hg.mozilla.org/mozilla-central/rev/82f73bdb2237

> * part 5: Back out bug 763689 part 2 (style-system handling for
> min-width:auto).
https://hg.mozilla.org/mozilla-central/rev/6bb37077d615a

> * part 6: Back out bug 763689 part 1 (CSS parser support for min-width:auto
> & min-height:auto).
https://hg.mozilla.org/mozilla-central/rev/ff0658329dbd

These may not need a very thorough review, since they're basically clean backouts, modulo bitrot, as noted above.  Let me know if I can provide any other info that'd make this review more straightforward.

Comment 13

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #12)
> For reference, I've pasted the original csets that are being backed out,
> below each patch-title, below:
[...]
> > * part 5: Back out bug 763689 part 2 (style-system handling for
> > min-width:auto).
> https://hg.mozilla.org/mozilla-central/rev/6bb37077d615a

Sorry, I accidentally appended an "a" at the end of that cset. The correct URL being backed out there is: https://hg.mozilla.org/mozilla-central/rev/6bb37077d615

Comment 14

[Daniel Holbert [:dholbert]]

Try run w/ all the attached patches (on Linux64 only):
 https://tbpl.mozilla.org/?tree=Try&rev=5dfc4b4f6f3b

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 727472 [details] [diff] [review]
part 1: Back out bug 794748 (mochitest for min-width:auto)

It's worth putting the original changesets in the commit messages.

Comment 16

[Daniel Holbert [:dholbert]]

Will do. Thanks!

Comment 17

[Daniel Holbert [:dholbert]]

Added original csets to commit messages, per review comment, and pushed:

part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/edd98255e81d
part 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/1eae876d6c3a
part 3: https://hg.mozilla.org/integration/mozilla-inbound/rev/7a289f49170f
part 4: https://hg.mozilla.org/integration/mozilla-inbound/rev/5db313632268
part 5: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8b5090d02e5
part 6: https://hg.mozilla.org/integration/mozilla-inbound/rev/e7a9e30409eb
part 7: https://hg.mozilla.org/integration/mozilla-inbound/rev/afe27d80eeef

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/edd98255e81d
https://hg.mozilla.org/mozilla-central/rev/1eae876d6c3a
https://hg.mozilla.org/mozilla-central/rev/7a289f49170f
https://hg.mozilla.org/mozilla-central/rev/5db313632268
https://hg.mozilla.org/mozilla-central/rev/f8b5090d02e5
https://hg.mozilla.org/mozilla-central/rev/e7a9e30409eb

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/afe27d80eeef

Comment 20

[Kohei Yoshino [:kohei]]

I've added this bug to the compatibility doc. Please correct the info if wrong.
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_22

Also, please update the following docs:
https://developer.mozilla.org/en-US/docs/CSS/min-width
https://developer.mozilla.org/en-US/docs/CSS/min-height

Comment 21

[Daniel Holbert [:dholbert]]

(In reply to Kohei Yoshino from comment #20)
> I've added this bug to the compatibility doc. Please correct the info if
> wrong.
> https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_22

Thanks! Looks good -- I just edited it to tweak the language & clarify (& linkify) a few things.

> Also, please update the following docs:
> https://developer.mozilla.org/en-US/docs/CSS/min-width
> https://developer.mozilla.org/en-US/docs/CSS/min-height

Hmm. I'm not actually sure what the new state of those docs should be.

I suspect we just remove all mention of "auto" (including compatibility charts) from that page, and add a note at the bottom saying that we temporarily supported "auto", in Firefox versions 18 - 21, because it was part of the flexbox spec for that period of time?  Or, do we tend to leave deprecated keywords in compat charts in situations like this, and mark them as deprecated somehow?

Comment 22

[Daniel Holbert [:dholbert]]

teoli, do you have any advice on how best to handle the min-width/min-height wiki pages' compat tables etc., in response to this value being removed (unsupported)?

Comment 23

[Daniel Holbert [:dholbert]]

Never mind, disregard comment 22 -- sheppy helped me out in IRC. I've updated the min-width and min-height MDN pages:
 - updated the initial value
 - removed auto from the lists of valid values
 - clarified the chunk about CSS flexbox being one of the specs that defines the property
 - Added "Obsolete since Gecko 22" badge to the "auto" lines in the compat table

Comment 24

[Daniel Holbert [:dholbert]]

And I think that makes this dev-doc-complete.

Comment 25

[Alex Keybl [:akeybl]]

No need to relnote - the feature has never been released (outside of Nightly/Aurora/Beta) in another form. See bug 841876.