939905 - Support "flex-flow" shorthand for multi-line flexbox

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Daniel Holbert [:dholbert]]

Bug 702508 adds support for "flex-wrap" property.

With that, we can now support the "flex-flow" shorthand property (which sets both "flex-direction" and "flex-flow").

http://dev.w3.org/csswg/css-flexbox/#flex-flow-property

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix v1

heycam, would you mind reviewing?

Link to spec is in comment 0. (and as implied by comment 0, this layers on the patches from that bug as well.)

Quoting the spec, for convenience:
> Name:	flex-flow
> Value:	<flex-direction> || <flex-wrap>

flex-direction and flex-wrap are both enum-valued properties. The "||" means they can be provided in either order, and either of them may be omitted (but at least one has to be there).

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 8334364 [details] [diff] [review]
fix v1

Actually, I think I might need to tweak the inherit/unset/initial handling a bit. Canceling review request for the moment.

Comment 5

[Cameron McCormack (:heycam)]

While I've begun looking at the patch, and before you put a new one up, I think it would be good to use ParseChoice to parse the two optional components, once you've got past the initial/inherit/unset checking.

Comment 6

[Cameron McCormack (:heycam)]

Actually, ParseChoice can handle the parsing of initial/inherit/unset, too.

Comment 7

[Daniel Holbert [:dholbert]]

Ah, that looks great - thanks! I'll give that a shot tomorrow. Looks like it should simplify things a good bit.

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: fix v2 (using ParseChoice)

Comment 9

[Daniel Holbert [:dholbert]]

https://tbpl.mozilla.org/?tree=Try&rev=3141d4d15d1d

Comment 10

[Cameron McCormack (:heycam)]

Comment on attachment 8334819 [details] [diff] [review]
fix v2 (using ParseChoice)

Looks great!  r=me with the understanding that bug 940229 will land soon and so we don't need to include invalid_values for initial/inherit/unset explicitly.

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to Cameron McCormack (:heycam) from comment #10)
> Comment on attachment 8334819 [details] [diff] [review]
> fix v2 (using ParseChoice)
> 
> Looks great!

Thanks!
> r=me with the understanding that bug 940229 will land soon and
> so we don't need to include invalid_values for initial/inherit/unset
> explicitly.

Yup. (I briefly added some invalid_values like that (which caused testfailures in my first patch), but I took it out after verifying that bug 940229 catches those issues just as effectively.)

Comment 12

[Daniel Holbert [:dholbert]]

Landed this (and the other helper-bugs for multi-line flexbox) on inbound:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/872e64c93fda

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/872e64c93fda