Support -webkit-box styling, as aliases for modern CSS flexbox (behind a pref)

RESOLVED FIXED in Firefox 45

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug)

Trunk
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 affected, firefox45 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Filing this bug on supporting properties related to "display:-webkit-box", as approximately-equivalent aliases for modern CSS flexbox.

(As noted in bug 1208344 comment 6, we can treat -webkit-box-[align|pack] as simple aliases for align-items & justify-content, once we support the 'start' & 'end' values for these properties, which are missing in the flexbox spec but have been added in css-align.  But -webkit-box-orient may require a bit more custom aliasing infrastructure, which I'm tentatively planning to provide in bug 1208344.)
(Assignee)

Comment 1

3 years ago
(This bug is about supporting these properties natively in CSS, as opposed to using CSSUnprefixingService.js like we do right now for a whitelist of sites.)
Daniel, does that mean that, when the work for align-items and justify-content 'start' and 'end' lands, we could make a prominent notice in a MDN page about -webkit-box-align and -webkit-box-pack advising authors to replace these by these both standard properties? 

If so, do you have a bug # for tracking the implementation of the start/end values? (My bugzilla abilities failed me to find this bug)
Flags: needinfo?(dholbert)
(Assignee)

Comment 3

3 years ago
(Essentially, "no".  Stepping back, we should be encouraging authors to use modern flexbox (and its suite of properties, including align-content/justify-content) instead of, or as fallback for, -webkit-box/-moz-box.  That's already true. But we shouldn't specifically ask authors to swap out only those 2 properties, regardless of "start"/"end" support.  In true webkit-based browsers, -webkit-box-align and -webkit-box-pack *only* have an effect on elements with display:-webkit-box, whereas align-items and justify-content have *no* effect on those elements.  So if authors swapped out *just* those two properties, it'd break their -webkit-box elements in webkit-based browsers.  It'd still work in Firefox, but that's only because our plan is to hackily support -webkit-box-* by mapping them under the hood to approximately-equivalent modern flexbox properties.

Bug 1176782 is where we're adding support for those justify-content/align-content values, though.)
Flags: needinfo?(dholbert)
Thanks! Sounds logical.
(Assignee)

Updated

3 years ago
Blocks: 1213126
(Assignee)

Comment 5

3 years ago
(In reply to Daniel Holbert [:dholbert] from comment #0 & #3)
> (As noted in bug 1208344 comment 6, we can treat -webkit-box-[align|pack] as
> simple aliases for align-items & justify-content, once we support the
> 'start' & 'end' values for these properties [...]
> Bug 1176782 is where we're adding support for those [...] values

Bug 1176782 has now landed (on inbound), BTW.
Depends on: 1176782
(Assignee)

Comment 6

3 years ago
Created attachment 8682796 [details] [diff] [review]
part 1 v1: add mappings for -webkit-box-flex, -webkit-box-ordinal-group, -webkit-box-align, and -webkit-box-pack

This patch adds support for several prefixed CSS properties associated with -webkit-box, as aliases for modern flexbox properties (and behind the pref "layout.css.prefixes.webkit").

Specifically, this adds the following alias mappings:
  -webkit-box-flex          --> flex-grow
  -webkit-box-ordinal-group --> order
  -webkit-box-align         --> align-items
  -webkit-box-pack          --> justify-content

Notable things *not* covered in this patch:
 - This patch does not add support for "-webkit-box-direction". That'll be a bit trickier, and that's covered in bug 1208344.
 - This patch does not add support for "display: -webkit-box". That'll be in a separate patch here.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8682796 - Flags: review?(cam)
(Assignee)

Comment 7

3 years ago
Created attachment 8682798 [details] [diff] [review]
part 2: Extend existing parser code for CSSUnprefixingService "-webkit-box" handling to also activate if native unprefixing is enable

This patch just extends some existing C++ code to check for the new pref along with the CSSUnprefixingService pref, and dutifully treat "display:-webkit-box" as "display:flex" in either case.
(Assignee)

Updated

3 years ago
Attachment #8682798 - Flags: review?(cam)
(Assignee)

Comment 8

3 years ago
Created attachment 8682799 [details] [diff] [review]
part 3: Match "-webkit-box" as a predefined keyword, not as a string

This patch is just an optimization for the code touched by part 2 -- it makes "-webkit-box" a predefined nsCSSKeyword enum, so we can discover it in our nsCSSKeywords::LookupKeyword call, instead of needing to do a special-case string-comparison.
Attachment #8682799 - Flags: review?(cam)
(Assignee)

Comment 9

3 years ago
(Er, the comment-tweak in part 3 really belongs in part 2. I'll make that change locally, but won't bother re-posting to avoid bug-churn.)
(Assignee)

Comment 10

3 years ago
(In reply to Daniel Holbert [:dholbert] from comment #6)
>  - This patch does not add support for "-webkit-box-direction". That'll be a
> bit trickier, and that's covered in bug 1208344.

(And of course I meant "-webkit-box-orient" there, not "-webkit-box-direction" [which is something completely different].)
Attachment #8682796 - Flags: review?(cam) → review+
Attachment #8682798 - Flags: review?(cam) → review+
Attachment #8682799 - Flags: review?(cam) → review+

Comment 13

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e0317d4a7a0d
https://hg.mozilla.org/mozilla-central/rev/8b1626ab504f
https://hg.mozilla.org/mozilla-central/rev/073a8c4b51ef
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
(Assignee)

Updated

3 years ago
Blocks: 1170789
(Assignee)

Updated

3 years ago
Blocks: 1231682
(Assignee)

Updated

3 years ago
Blocks: 1234941
(Assignee)

Comment 14

3 years ago
(Taking this off of the dependency list for bug 1170774, since it's already in the dependency tree via bug 1170789.)
No longer blocks: 1170774
(Assignee)

Updated

3 years ago
Depends on: 1238668
Comment hidden (typo)
(Assignee)

Comment 16

3 years ago
Sorry, disregard comment 15; I mis-pasted the bug number into a patch for bug 1257661.
(Assignee)

Updated

3 years ago
Attachment #8731907 - Attachment is obsolete: true
Attachment #8731907 - Flags: review?(cam)
(Assignee)

Updated

3 years ago
Blocks: 1257688
(Assignee)

Updated

3 years ago
No longer blocks: 1257688

Updated

2 years ago
Depends on: 1277257
Keywords: dev-doc-needed
I just realized that these properties were changed later on in bug 1257688 to map to the -moz prefixed properties.
I need to update them accordingly.

Just to be clear, in Gecko 45 to 47 they mapped to the modern flexbox properties and from 48 on they map to the ones listed above, correct? And if so, should this circumstance actually be reflected in the modern flexbox properties pages?

Sebastian
Blocks: 1257688
(Assignee)

Comment 19

2 years ago
(In reply to Sebastian Zartner [:sebo] from comment #18)
> Just to be clear, in Gecko 45 to 47 they mapped to the modern flexbox
> properties and from 48 on they map to the ones listed above, correct?

Correct.

> And if
> so, should this circumstance actually be reflected in the modern flexbox
> properties pages?

I don't think there's any compelling reason to document that subtlety.  (The 45-47 behavior is disabled-by-default, so not something anyone should be depending on [not that anyone should depend on these anyway, sigh].  And in practice, the two versions of the behavior basically behave the same.)

Anyway -- I think the language you added is appropriately vague about the details. :) (in the pages linked in comment 17) Looks good to me. Thanks again!
Flags: needinfo?(dholbert)
Thank you for the feedback!

(In reply to Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th) from comment #19)
> (In reply to Sebastian Zartner [:sebo] from comment #18)
> > And if
> > so, should this circumstance actually be reflected in the modern flexbox
> > properties pages?
> 
> I don't think there's any compelling reason to document that subtlety.  (The
> 45-47 behavior is disabled-by-default, so not something anyone should be
> depending on [not that anyone should depend on these anyway, sigh].  And in
> practice, the two versions of the behavior basically behave the same.)
> 
> Anyway -- I think the language you added is appropriately vague about the
> details. :) (in the pages linked in comment 17) Looks good to me. Thanks
> again!

Ok, so I've updated them to say 48 instead of 45.

Sebastian
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.