1122253 - adjust nsRuleNode hack so that height:min-content etc. keywords work in vertical writing modes

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

In bug 1117983 parts 1, 2 and 3, I make {inline,block}-size parse the keywords that width takes -- min-content, -moz-available, etc. -- but convert them to unset if they happen to get set on height.  We should undo this change to nsRuleNode::ComputePositionData once we support these keywords on height.

Comment 1

[Simon Montagu :smontagu]

(In reply to Cameron McCormack (:heycam) (away April 3-6) from comment #0)
> We should undo
> this change to nsRuleNode::ComputePositionData once we support these
> keywords on height.

This isn't quite right IMHO: we don't want to support these keywords on height as such, but rather on inline-size (whether width or height) and not on block-size (whether height or width). Bug 1106669 already made nsLayoutUtils::IntrinsicForContainer work on inline-size instead of width, so I'm not sure that this hack was ever necessary.

Comment 2

[Cameron McCormack (:heycam)]

So rather than remove the hack, I think we just need to adjust it.  Per some email conversation, I think we want to do this:

> 1. adjust the “hack” to allow the keywords to be set on
>    nsStylePosition::mHeight (and prevent them being set on mWidth)
>    depending on the writing mode, rather than always allowing them on
>    mWidth and not on mheight
> 
> 2. allow the keywords to be parsed as ‘height’ property values
> 
> 3. disallow the keywords on ‘block-size’

Comment 3

[Cameron McCormack (:heycam)]

Attachment: Support -moz-min-content etc. keywords in {,min-,max-}inline-size and their physical equivalents.

Comment 4

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0d8af758a88

Comment 5

[Daniel Holbert [:dholbert]]

(heads-up: I'm backlogged on reviews at the moment [catching up, but still have some big grid ones to get through].  If you can find another reviewer, you may have have a faster review turnaround.)

Comment 6

[Cameron McCormack (:heycam)]

Comment on attachment 8624596 [details] [diff] [review]
Support -moz-min-content etc. keywords in {,min-,max-}inline-size and their physical equivalents.

OK, thanks for the heads up Daniel.  bz you good to take this one?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8624596 [details] [diff] [review]
Support -moz-min-content etc. keywords in {,min-,max-}inline-size and their physical equivalents.

>+  const bool vertical = aContext->StyleVisibility()->mWritingMode !=
>+                          NS_STYLE_WRITING_MODE_HORIZONTAL_TB;

Could you MOZ_ASSERT that the value is one of the ones we know about (so we'll catch it if we add a new value)?

r=me with that.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/faf18dc3d5a1

Comment 9

[Carsten Book [:Tomcat]]

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