Enable layout.css.prefixes.webkit by default (though this was later restricted to non-release builds, in bug 1238827)

RESOLVED FIXED in Firefox 46

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
2 years ago
4 months ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Depends on: 4 bugs, Blocks: 2 bugs, {dev-doc-needed})

Trunk
mozilla46
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 unaffected, firefox46 fixed, relnote-firefox 47+)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Filing this bug on enabling the "layout.css.prefixes.webkit" pref by default.

At this point, I think the main things blocking that are -webkit-box styling (bug 1208635) and webkit gradients (bug 1132748).  There may be other webkit prefixed properties that we add later on, but those don't need to block the rest of them from being enabled.

(Really, we could probably enable this now and start testing what's already in.  But I'd rather not quite yet, because having this turned off means we can land -webkit-box & prefixed gradient support in incremental pieces, behind the pref, without having to worry about the intermediate states causing breakage on random sites.)

(Also not sure yet if this will be a "enable it, period" bug or an "enable it on Nightly/Aurora" bug. Might start with the former, and then we can always pref it off on branches if we discover problems & need to hold it for a release or two.)
(In reply to Daniel Holbert [:dholbert] (less responsive Oct 9-12 & 15-18) from comment #0)
> [...] having this turned off means we
> can land -webkit-box & prefixed gradient support in incremental pieces,
> behind the pref, without having to worry about the intermediate states
> causing breakage on random sites.)

This sounds sensible.
 
> (Also not sure yet if this will be a "enable it, period" bug or an "enable
> it on Nightly/Aurora" bug. Might start with the former, and then we can
> always pref it off on branches if we discover problems & need to hold it for
> a release or two.)

Also SGTM.
(Assignee)

Updated

2 years ago
Blocks: 1170774
(Assignee)

Comment 2

2 years ago
(Sorry, all mentions of "bug 1132748" here were really supposed to say "bug 1210575". The former was for CSSUnprefixingService-backed gradient support; the latter is for native support.)
Depends on: 1210575
No longer depends on: 1132748
(Assignee)

Updated

2 years ago
Depends on: 1217643
Duplicate of this bug: 1177263
(Assignee)

Updated

a year ago
Depends on: 1208344
Keywords: dev-doc-needed
(Assignee)

Comment 4

a year ago
Created attachment 8703064 [details] [diff] [review]
patch v1: flip the pref

Here's the patch. I think this is ready to land, once bug 1208344 is in.

As noted at the end of comment 0, I'm optimistically hoping this can just ride the trains -- hence, no ifndef MOZ_RELEASE guard here -- but we can always pref it off on a release channel if we discover some fallout.
Attachment #8703064 - Flags: review?(cam)
(Assignee)

Comment 5

a year ago
(Note that I'm not bothering to pref off the js-based CSS Unprefixing Service pref here -- per bug 1210575 part 4, this newer pref will disable the CSS Unprefixing Service the hood anyway. And if we (or users) want to revert this bug's change, there's only one pref to flip to restore the old behavior, instead of two.  Once this new impl has stuck, we can just rip out the unprefixing service entirely.)
(Assignee)

Comment 6

a year ago
*under the hood
Comment on attachment 8703064 [details] [diff] [review]
patch v1: flip the pref

Review of attachment 8703064 [details] [diff] [review]:
-----------------------------------------------------------------

Hooray!
Attachment #8703064 - Flags: review?(cam) → review+
\o/, thanks for all the work you put in to make this happen Daniel and Cameron!
(Assignee)

Comment 9

a year ago
Likewise!

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d66cd0ec433

Comment 10

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9fbf850dc78d

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9fbf850dc78d
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46

Updated

a year ago
Depends on: 1236506
Blocks: 1236928
Depends on: 1236930
Depends on: 1237101
Depends on: 1238527

Updated

a year ago
Depends on: 1238580

Updated

a year ago
Depends on: 1238879
No longer depends on: 1238879
Depends on: 1239342
No longer depends on: 1239342
(Assignee)

Updated

a year ago
Depends on: 1239136
Marking 44 unaffected (since this landed in 46)
status-firefox44: affected → unaffected

Updated

a year ago
Depends on: 1240611
Noting this is moving to aurora soon but will stay on aurora (bug 1238827)

Updated

a year ago
Depends on: 1241588

Updated

a year ago
Depends on: 1242333
(Assignee)

Updated

a year ago
Blocks: 1238827
Release Note Request (optional, but appreciated)
[Why is this notable]: Important change for compatibility
[Suggested wording]: Improved Web compatibility by supporting WebKit CSS prefixed properties (via layout.css.prefixes.webkit)
[Links (documentation, blog post, etc)]: Do we have an official blog post which explains that?
relnote-firefox: --- → ?
Depends on: 1243654
(Assignee)

Updated

a year ago
No longer depends on: 1242333
Depends on: 1244464
This is noted for 46 as "Added support for WebKitCSSMatrix to improve mobile Web compatibility"

Can you suggest a link for the release notes? (Daniel - I can always add in the link next week when you are back, no rush)
relnote-firefox: ? → 46+
Flags: needinfo?(dholbert)
(Assignee)

Comment 16

a year ago
Webkit-prefixed features are not actually shipping past Aurora right now (as of bug 1238827). So, we probably want to remove that relnote for 46beta & 46release.
Flags: needinfo?(dholbert)
Oh, right! Thanks. I have removed it. My note was actually for bug 717722, "Added support for WebKitCSSMatrix to improve mobile Web compatibility".  I think these are basically the same note, and that bug also mentions staying in aurora. 

Ritu, should we move the note to 47?
relnote-firefox: 46+ → 47+
Flags: needinfo?(rkothari)
Done. I added it to Fx47 (aurora) release notes.
Flags: needinfo?(rkothari)

Updated

a year ago
Depends on: 1256664

Updated

a year ago
Depends on: 1258733

Updated

a year ago
Depends on: 1259437
(Assignee)

Updated

a year ago
No longer depends on: 1259437

Updated

a year ago
Depends on: 1265745
(Assignee)

Updated

a year ago
Summary: Enable layout.css.prefixes.webkit by default → Enable layout.css.prefixes.webkit by default (later restricted to non-release builds)
(Assignee)

Updated

a year ago
Summary: Enable layout.css.prefixes.webkit by default (later restricted to non-release builds) → Enable layout.css.prefixes.webkit by default (though this was later restricted to non-release builds, in bug 1238827)
(Assignee)

Comment 19

a year ago
Liz, looks like this is still in the Firefox 46 release notes ("Gecko now accepts the -webkit- prefixed version of some properties;")[1], and it's caused some confusion on hacker news[2].

Someone already edited the release notes to clarify that you have to tweak the pref, but we probably should just remove this piece of the release notes entirely, right?  I don't think we normally announce disabled-by-default features there. (And I thought we had removed it already in earlier versions of 46 release notes, per comment 17.)  I'm guessing this release-notes inclusion was just an accident.

[1] https://developer.mozilla.org/en-US/Firefox/Releases/46
[2] https://news.ycombinator.com/item?id=11579148
Flags: needinfo?(lhenry)
I don't work on the MDN developer release notes - only on what we put onto mozilla.org for the main Firefox release notes.
jypenator -- I think that it is probably best to move this note and similar ones to the 47 beta notes, since they aren't yet enabled by default.
Flags: needinfo?(lhenry) → needinfo?(jypenator)
(Assignee)

Comment 21

a year ago
Thanks for the clarification! (Note that these webkit prefixing features aren't enabled in 47 beta either, actually -- so 47 beta notes isn't a good place for them either.  They're tentatively slated to be enabled in 48beta+release or 49beta+release -- that's bug 1259345.)
(In reply to Daniel Holbert [:dholbert] from comment #19)
> Someone already edited the release notes to clarify that you have to tweak
> the pref, but we probably should just remove this piece of the release notes
> entirely, right?  I don't think we normally announce disabled-by-default
> features there.

These notes are for developers, so we *do* list disabled-by-default features (with a hint for the preference to enable them).

Sebastian
Flags: needinfo?(jypenator)
(Assignee)

Updated

a year ago
Blocks: 1229757
(Assignee)

Updated

11 months ago
Blocks: 1277758
(Assignee)

Updated

10 months ago
Blocks: 1171872

Updated

7 months ago
Depends on: 1307118

Updated

6 months ago
Blocks: 1311070

Updated

6 months ago
Depends on: 1312918

Updated

5 months ago
Depends on: 1322341
Depends on: 1314051
You need to log in before you can comment on or make changes to this bug.