1186636 - Add pref to control legacy -moz-prefixed gradients

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Mike Taylor [:miketaylr]]

This should be enabled by default, but will allow people to test sites with legacy prefixed gradients disabled.

dholbert suggests "layout.css.moz-gradient.enabled" in https://bugzilla.mozilla.org/show_bug.cgi?id=1176496#c18.

Comment 1

[Daniel Holbert [:dholbert]]

emk, would you like to take this?

(miketaylr also volunteered in IRC, so no pressure if you're busy with other things.)

I think all we need here is a new static bool, a targeted check for it, and a default value in all.js, as discussed in bug 1176496 comment 18.

> dholbert suggests "layout.css.moz-gradient.enabled"

I'm not married to this name, FWIW, in part because "moz-gradient" isn't actually a thing; it's just an abbreviation for moz-[radial|linear]-gradient. If we come up with something better/clearer, that's fine too. The name probably doesn't matter too much, because it's mostly intended for our own testing.

(I initially considered "legacy-gradient" or "prefixed-gradient", but those are likely more confusing since we will soon globally support legacy/prefixed "-webkit-*-gradient" variants, and that support won't be related to this pref.)

Comment 2

[Masatoshi Kimura [:emk]]

IMO it's better to add separate prefs for -moz-linear-gradient and -moz-radial-gradient such as "layout.css.moz-linear-gradient.enabled" and "layout.css.moz-radial-gradient.enabled".
Because -moz-radial-gradient usage would be much less than -moz-linear-gradient usage. All bug 1176496 blocker depended on -moz-linear-gradient.

Comment 3

[Masatoshi Kimura [:emk]]

Ah, I forgot to response the question. Taking.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to Masatoshi Kimura [:emk] from comment #2)
> IMO it's better to add separate prefs [....]
> Because -moz-radial-gradient usage would be much less than -moz-linear-gradient usage.

Hmm... I'm mixed on this idea. So the motiviation here is to allow us to unsupport -moz-radial-gradient sooner, while keeping -moz-linear-gradient around for longer, since more of the web depends on it?

(I think you're right about -moz-radial-gradient being less frequent -- but I'd bet that's just because radial gradients are less common *in general* than linear gradients. It doesn't necessarily mean the websites *that do* use radial gradients are any more likely to use them in a standards-compatible way.)

> All bug 1176496 blocker depended on -moz-linear-gradient.

This is an interesting observation -- but I expect it's just because radial gradients are less common, and because bug 1176496 only lived on trunk for a few days. If we'd kept it on Nighlty for longer, I expect we would have found some sites that rely on -moz-radial-gradient & were broken, too.

Comment 5

[Daniel Holbert [:dholbert]]

(Anyway, since the pref/prefs are just for our own nightly testing at this point anyway, I shouldn't bikeshed the number-of-prefs decision too much. :) So, I'm OK with proceeding down the two-pref path at this point.)

Comment 6

[Mike Taylor [:miketaylr]]

From a developer point of view, it would be nice to only have to flip one pref to test my site for old-gradient bustage (if we think devs will actually do this).

Comment 7

[Masatoshi Kimura [:emk]]

Attachment: Add a pref to configure -moz prefixed gradients support

OK, I'm adding only one pref for now.

I employed "layout.css.prefixes.gradients" for consistency with existing pref names. If this name is confusing, we should rename all similar prefs at once.

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 8637878 [details] [diff] [review]
Add a pref to configure -moz prefixed gradients support

Looks great, thanks!

(I hadn't realized that we had an existing convention on naming for -moz prefix prefs -- thanks for noticing/knowing that & staying consistent with it.)

Comment 9

[Daniel Holbert [:dholbert]]

Comment on attachment 8637878 [details] [diff] [review]
Add a pref to configure -moz prefixed gradients support

er, r+.

Comment 10

[Kohei Yoshino [:kohei]]

Why not to enable the pref by default on the aurora and nightly channels?

Comment 11

[Kohei Yoshino [:kohei]]

I meant: *disable* (layout.css.prefixes.gradients = false)

Comment 12

[Daniel Holbert [:dholbert]]

That's a separate decision than simply making it pref-controllable.

It'll also require a more substantial patch, because it'll require us to drop prefixed gradients [or tweaking the pref] in all our automated testcases, or else we'll have test failures on nightly (and eventually aurora) TreeHerder.

So, probably better to perform that change in its own bug. (Probably worth doing, though.)

Comment 13

[Daniel Holbert [:dholbert]]

(Also: as long as we support this pref for release users, we do need to keep *some* testcases that ensure that they actually work. So we don't want to remove prefixes from *all* our tests, like we did in bug 1176496.)

Comment 14

[Kohei Yoshino [:kohei]]

Okay, thanks. I forgot about automated tests.

Comment 15

[Masatoshi Kimura [:emk]]

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

Comment 16

[Pulsebot]

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

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 18

[Jean-Yves Perrier [:teoli]]

I've added a tiny note in the compat data tables about the existence of this pref:
https://developer.mozilla.org/en-US/docs/Web/CSS/linear-gradient#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/CSS/radial-gradient#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/CSS/repeating-linear-gradient#Browser_Compatibility
https://developer.mozilla.org/en-US/docs/Web/CSS/repeating-radial-gradient#Browser_compatibility
and
https://developer.mozilla.org/en-US/Firefox/Releases/42#CSS