The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla22

Status

()

Core
Layout
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 2 bugs, {dev-doc-complete, site-compat})

Trunk
mozilla22
dev-doc-complete, site-compat
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox -)

Details

Attachments

(7 attachments)

(Assignee)

Description

4 years ago
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
(Assignee)

Updated

4 years ago
Keywords: dev-doc-complete
(Assignee)

Updated

4 years ago
Keywords: dev-doc-complete → dev-doc-needed
(Assignee)

Comment 1

4 years ago
(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
(Assignee)

Updated

4 years ago
Summary: Remove support for "min-width:auto" and "min-height: auto", if they're dropped from flexbox spec → Remove support for "min-width:auto" and "min-height: auto", since they're being dropped from flexbox spec
(Assignee)

Comment 2

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

Comment 3

4 years ago
(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.
(Assignee)

Comment 4

4 years ago
Created attachment 727472 [details] [diff] [review]
part 1: Back out bug 794748 (mochitest for min-width:auto)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #727472 - Flags: review?(dbaron)
(Assignee)

Comment 5

4 years ago
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)
Attachment #727473 - Flags: review?(dbaron)
(Assignee)

Comment 6

4 years ago
(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.)
(Assignee)

Comment 7

4 years ago
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)
Attachment #727475 - Flags: review?(dbaron)
(Assignee)

Comment 8

4 years ago
Created attachment 727476 [details] [diff] [review]
part 4: Back out bug 763689 part 3 (style-system handling for min-height:auto).
Attachment #727476 - Flags: review?(dbaron)
(Assignee)

Comment 9

4 years ago
Created attachment 727477 [details] [diff] [review]
part 5: Back out bug 763689 part 2 (style-system handling for min-width:auto).
Attachment #727477 - Flags: review?(dbaron)
(Assignee)

Comment 10

4 years ago
Created attachment 727478 [details] [diff] [review]
part 6: Back out bug 763689 part 1 (CSS parser support for min-width:auto & min-height:auto)
Attachment #727478 - Flags: review?(dbaron)
(Assignee)

Comment 11

4 years ago
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")
Attachment #727487 - Flags: review?(dbaron)
(Assignee)

Comment 12

4 years ago
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.
(Assignee)

Comment 13

4 years ago
(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
(Assignee)

Comment 14

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

Updated

4 years ago
Summary: 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 being dropped from flexbox spec
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.
Attachment #727472 - Flags: review?(dbaron) → review+
Attachment #727473 - Flags: review?(dbaron) → review+
Attachment #727475 - Flags: review?(dbaron) → review+
Attachment #727476 - Flags: review?(dbaron) → review+
Attachment #727477 - Flags: review?(dbaron) → review+
Attachment #727478 - Flags: review?(dbaron) → review+
Attachment #727487 - Flags: review?(dbaron) → review+
(Assignee)

Comment 16

4 years ago
Will do. Thanks!
(Assignee)

Comment 17

4 years ago
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
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
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
https://hg.mozilla.org/mozilla-central/rev/afe27d80eeef
relnote-firefox: --- → ?
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
(Assignee)

Comment 21

4 years ago
(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?
(Assignee)

Comment 22

4 years ago
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)?
Flags: needinfo?(jypenator)
(Assignee)

Comment 23

4 years ago
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
Flags: needinfo?(jypenator)
(Assignee)

Comment 24

4 years ago
And I think that makes this dev-doc-complete.
Keywords: dev-doc-needed → dev-doc-complete
No need to relnote - the feature has never been released (outside of Nightly/Aurora/Beta) in another form. See bug 841876.
relnote-firefox: ? → -

Updated

4 years ago
Keywords: site-compat
(Assignee)

Updated

3 years ago
Depends on: 666041
(Assignee)

Updated

3 years ago
Blocks: 984711
(Assignee)

Updated

3 years ago
Blocks: 1055888
You need to log in before you can comment on or make changes to this bug.