Last Comment Bug 848539 - Remove support for "min-width: auto" and "min-height: auto", since they're being dropped from flexbox spec
: Remove support for "min-width: auto" and "min-height: auto", since they're be...
Status: RESOLVED FIXED
: dev-doc-complete, site-compat
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla22
Assigned To: Daniel Holbert [:dholbert]
:
Mentors:
Depends on: css3-flexbox 763689
Blocks: 788358 flexbox-spec-changes 841876 984711
  Show dependency treegraph
 
Reported: 2013-03-06 13:34 PST by Daniel Holbert [:dholbert]
Modified: 2014-08-19 18:45 PDT (History)
7 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
part 1: Back out bug 794748 (mochitest for min-width:auto) (8.36 KB, patch)
2013-03-20 18:31 PDT, Daniel Holbert [:dholbert]
dbaron: review+
Details | Diff | Splinter Review
part 2: Back out bug 666041 part 8 (special-handling for min-width:auto as min-content, for flex items) (3.68 KB, patch)
2013-03-20 18:32 PDT, Daniel Holbert [:dholbert]
dbaron: review+
Details | Diff | Splinter Review
part 3: Back out part of bug 666041 part 7 (flexbox frame class impl -- just removing the code that deals with min-height:auto) (4.81 KB, patch)
2013-03-20 18:37 PDT, Daniel Holbert [:dholbert]
dbaron: review+
Details | Diff | Splinter Review
part 4: Back out bug 763689 part 3 (style-system handling for min-height:auto). (10.54 KB, patch)
2013-03-20 18:38 PDT, Daniel Holbert [:dholbert]
dbaron: review+
Details | Diff | Splinter Review
part 5: Back out bug 763689 part 2 (style-system handling for min-width:auto). (13.61 KB, patch)
2013-03-20 18:38 PDT, Daniel Holbert [:dholbert]
dbaron: review+
Details | Diff | Splinter Review
part 6: Back out bug 763689 part 1 (CSS parser support for min-width:auto & min-height:auto) (5.05 KB, patch)
2013-03-20 18:39 PDT, Daniel Holbert [:dholbert]
dbaron: review+
Details | Diff | Splinter Review
part 7: Fix or disable reftests that depend on min-width:auto / min-height:auto. (35.73 KB, patch)
2013-03-20 19:00 PDT, Daniel Holbert [:dholbert]
dbaron: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2013-03-06 13:34:52 PST
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] 2013-03-14 13:00:13 PDT
(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] 2013-03-14 20:27:44 PDT
(and the ED of the spec has been updated: https://dvcs.w3.org/hg/csswg/rev/9437131b3d6e )
Comment 3 Daniel Holbert [:dholbert] 2013-03-20 18:14:27 PDT
(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] 2013-03-20 18:31:22 PDT
Created attachment 727472 [details] [diff] [review]
part 1: Back out bug 794748 (mochitest for min-width:auto)
Comment 5 Daniel Holbert [:dholbert] 2013-03-20 18:32:12 PDT
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)
Comment 6 Daniel Holbert [:dholbert] 2013-03-20 18:36:52 PDT
(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] 2013-03-20 18:37:36 PDT
Created attachment 727475 [details] [diff] [review]
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] 2013-03-20 18:38:19 PDT
Created attachment 727476 [details] [diff] [review]
part 4: Back out bug 763689 part 3 (style-system handling for min-height:auto).
Comment 9 Daniel Holbert [:dholbert] 2013-03-20 18:38:53 PDT
Created attachment 727477 [details] [diff] [review]
part 5: Back out bug 763689 part 2 (style-system handling for min-width:auto).
Comment 10 Daniel Holbert [:dholbert] 2013-03-20 18:39:46 PDT
Created attachment 727478 [details] [diff] [review]
part 6: Back out bug 763689 part 1 (CSS parser support for min-width:auto & min-height:auto)
Comment 11 Daniel Holbert [:dholbert] 2013-03-20 19:00:04 PDT
Created attachment 727487 [details] [diff] [review]
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] 2013-03-20 19:05:23 PDT
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] 2013-03-20 19:07:52 PDT
(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] 2013-03-20 23:24:27 PDT
Try run w/ all the attached patches (on Linux64 only):
 https://tbpl.mozilla.org/?tree=Try&rev=5dfc4b4f6f3b
Comment 15 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2013-03-27 18:56:01 PDT
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] 2013-03-27 22:36:27 PDT
Will do. Thanks!
Comment 19 Ryan VanderMeulen [:RyanVM] 2013-03-28 05:36:39 PDT
https://hg.mozilla.org/mozilla-central/rev/afe27d80eeef
Comment 20 Kohei Yoshino [:kohei] 2013-03-29 09:51:47 PDT
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] 2013-03-29 11:03:25 PDT
(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] 2013-03-29 11:51:30 PDT
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] 2013-03-29 15:59:25 PDT
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] 2013-03-29 15:59:52 PDT
And I think that makes this dev-doc-complete.
Comment 25 Alex Keybl [:akeybl] 2013-04-04 15:48:21 PDT
No need to relnote - the feature has never been released (outside of Nightly/Aurora/Beta) in another form. See bug 841876.

Note You need to log in before you can comment on or make changes to this bug.