A few months ago I wrote some patches to support some -webkit- prefixed CSS properties in order to help with analysis of Web sites that weren't functioning correctly in Firefox for Android (i.e., figuring out whether certain failures were solely due to -webkit- prefixed properties). It turns out that this is useful for debugging site problems as well; the easiest way to test whether a particular site problem is due to -webkit- prefixes or due to other reasons is to see what happens if those -webkit- prefixes are supported. Likewise, it may also be useful for further analysis work like the analysis we did a few months ago. So in order to help debug Web sites that aren't functioning correctly in Firefox and analyze how many of the problems are due to different -webkit- prefixed technology (or combinations thereof), I'd like to land this patch in mozilla-central, behind an off-by-default preference. This would allow those who need to test -webkit- prefix support to simply flip the pref in order to test the behavior of a page.

I have concerns with exposing such support in a release, even behind an off-by-default pref. Consider this scenario: an addon that sets the pref on by default is released (under whichever name variation or camouflage possible: "Webkit Firefox", "Compatible Fox", etc.). This addon gets increasingly popular due to the lack of public knowledge regarding what vendor prefixes are and the simple perceived effect that "it fixes firefox". The end situation is a presumably redundant off-by-default pref and a consistent "meh, -webkit actually *can* and *does* work anywhere" mentality. I very much agree that the above is a pessimistic scenario, but plausible. Restricting the functionality up until Aurora (I wouldn't go as far as Beta, even) is safer if such a patch is ever going to land.

(In reply to Victor Porof [:vp] from comment #1) > I have concerns with exposing such support in a release, even behind an > off-by-default pref. Consider this scenario: an addon that sets the pref on > by default is released (under whichever name variation or camouflage > possible: "Webkit Firefox", "Compatible Fox", etc.). This addon gets > increasingly popular due to the lack of public knowledge regarding what > vendor prefixes are and the simple perceived effect that "it fixes firefox". > The end situation is a presumably redundant off-by-default pref and a > consistent "meh, -webkit actually *can* and *does* work anywhere" mentality. > > I very much agree that the above is a pessimistic scenario, but plausible. > Restricting the functionality up until Aurora (I wouldn't go as far as Beta, > even) is safer if such a patch is ever going to land. Certainly, I think that's a valid concern. The only need for site investigation is an ability to do it, but it's not needed on a release channel. In fact, we could have this pref available for Nightly only. That would be sufficient and not take the risks you note.

Yeah, I was thinking about something like that, though I was concerned about the risk of ending up with something that doesn't compile when it's merged to Beta on merge day.

Could this be done outside of the browser? With some kind of prefix-free like bookmarklet?

(In reply to hsablonniere from comment #4) > Could this be done outside of the browser? With some kind of prefix-free > like bookmarklet? Not with anywhere near the same accuracy (e.g., most interactions with script).

bool PrefEnabled() { #ifdef MOZ_RELEASE_BUILD return false; #else return Preferences::GetBool("layout.css.webkit-prefix.enabled"); #endif } would be safe enough.

Note that this does not emulate the WebKit quirk of supporting element.style["-webkit-animation"] or supporting the uppercase element.style.WebkitAnimation (etc.) as opposed to the lowercase (and enumerable) element.style.webkitAnimation. Note that this also does not add aliases for transition or animation events.

Comment on attachment 733679 [details] [diff] [review] patch 2: Add flags field to CSS_PROP_ALIAS macro. r=me

Comment on attachment 733680 [details] [diff] [review] patch 3: Add flag to control whether CSS properties are enabled (applicable to aliases as well), so that properties can be disabled without substantive code change. >+ raise StandardError("unexpected flag" + flag) You probably want a space before the closing '"'. r=me

Comment on attachment 733681 [details] [diff] [review] patch 4: Off-by-default preference, available only in nightly/aurora versions, for testing the effect of supporting -webkit- prefixes or unprefixed properties. r=me

Comment on attachment 733682 [details] [diff] [review] patch 5: Add test that -webkit- prefixes are unsupported. r=me

Comment on attachment 733678 [details] [diff] [review] patch 1: Add ALPHA_BUILD as the opposite of RELEASE_BUILD so we can be careful about having included the definitions we want, and define MOZ_RELEASE_BUILD and MOZ_ALPHA_BUILD in C++. Review of attachment 733678 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +3911,5 @@ > GRE_MILESTONE=`tail -n 1 "$_topsrcdir"/config/milestone.txt 2>/dev/null || tail -1 "$_topsrcdir"/config/milestone.txt` > fi > AC_SUBST(GRE_MILESTONE) > > +if echo "$GRE_MILESTONE" | grep -q "a"; then We should probably just consolidate this and the rules.mk logic. @@ +3914,5 @@ > > +if echo "$GRE_MILESTONE" | grep -q "a"; then > + AC_DEFINE(MOZ_ALPHA_BUILD) > +else > + AC_DEFINE(MOZ_RELEASE_BUILD) Can we make these match the PREF_PPFLAGS versions? Having one with a MOZ_ prefix and the other without seems confusing.

Hi, what is the status of this bug? Is this already checked into nightly builds? If not, any ETA? Thanks.

(In reply to Sunil from comment #17) > Hi, what is the status of this bug? Is this already checked into nightly > builds? If not, any ETA? > > Thanks. It is a work in progress as per the review above. There is nothing checked in.

Hi, Is there any interest in taking this to completion just to see how many sites might get fixed in Firefox OS? I am more looking at Gmail to begin with. Moreover, there's a comment in one of the attachments 'Note that this also does not add aliases for transition or animation events.' Is there any guidance how to go about doing that if I want to play with the patch and I can report back. Thanks.

My current inclination is not to land the patches; apologies for wasting code reviewers' time on them. There seemed to me to be less interest in doing this sort of testing, and I'm also more concerned about other parties enabling -webkit- prefixes without talking to us if the patch were in. (I'd also need to post a revised patch 1 for re-review; it's in my patch queue, though.)

IMHO, if this patch was in (and available as part of the nightly builds behind a preference, hence beyond alpha builds), then tech evangelism might become easier as the guilty party can quickly see that if they added support for either non prefixed/moz prefixed CSS prop, then their web app can work on Firefox OS and Fennec too (I know reality is never this easy, but atleast webkit only app developers can quickly try their app on Fennec). And potentially also add support for webkit transition and animation events, for the same reason as above. In any case, will appreciate pointers on how to add alias for webkit transition and animation events so that I can do some testing. Thanks.

dbaron's been good enough to keep his patches for this bug unbitrotted in his patch queue. I'd like to land them, in support of bug 1170774 (for better compatibility with the web-as-it-stands, particularly on mobile). I'm planning on doing the following: (1) Obsoleting patch 1, since we don't want any ALPHA_BUILD dependency for these aliases anymore -- we're planning on shipping them in release builds. (2) Posting up-to-date versions of patches 2-5 (taken from dbaron's patch queue) on this bug. (3) Sanity-checking the differences between those up-to-date patches vs. the reviewed patches here, and (with dbaron's blessing) fixing up any issues that I notice. (4) Landing these patches on dbaron's behalf [with dbaron as patch-author] and closing out this bug. dbaron, I'm pretty sure you're on board with this, but let me know if you have any thoughts/concerns.

Changes in the updated version of this patch all look good. The changes I'm seeing from diffing this patch vs. its old version on this bug are as follows: (1) DO_PROP has been changed (in some other bug) to take 'flags', so this patch-version just directly passes 'flags' through to it, which makes sense. (In the old patch series, this didn't happen until part 3.) (2) CSS_PROP_ALIAS expressions are already multi-line now, so this patch doesn't have to make them multi-line anymore. (In the old patch version, the replaced expressions were giant one-liners.) (3) We have more CSS_PROP_ALIAS expressions than we used to. (And we're adding a 0 flags value to each of them.) These diffs all seem sensible.

Changes in the updated version of this patch (ignoring contextual differences: - Typo fix in some new python code (s/false/False/ & whitespace fix) - No need to add 'flags' anymore since it's already present, as noted above. - The numeric value of CSS_PROPERTY_DISABLED_BY_DEFAULT is larger, since its previous value has been taken. All good.

The only non-contextual change in this patch is that MOZ_ALPHA_BUILD has been renamed to ALPHA_BUILD, which is unimportant because we're not going to use that #define anyway, per above notes about skipping patch 1. I made one change to this patch -- I edited the commit message to remove the mention of nightly/aurora. I'll post an interdiff with my own minor code-tweaks (mostly removing that nightly/alpha restriction) which I plan to fold into this patch when landing.

Here are my tweaks to part 4, which I plan to fold in before landing: - I removed the ALPHA_BUILD switch (leaving behind the formerly-ALPHA_BUILD-only code). - I moved the -webkit aliases to the bottom nsCSSPropAliasList.h (where they were, in the previously-reviewed patch version here), so that we don't end up with -moz aliases followed by -webkit aliases followed by more -moz aliases. (Let me know if you meant for the order to be as it was in your unbitrotted version -- I'm assuming it wasn't an explicit choice & was just an artifact of unbitrotting.) Note: for simplicity, I'm keeping the preference name ("layout.css.test-webkit-prefixes") unchanged for now, but we'll probably want to rename it to something more definitive-sounding before this pref ships.

Changes in the updated version of this patch: - updated to modern mochitest manifest-file. (Note: since ultimately we *will* want to support these properties by default, we'll need to tweak this mochitest at that point. Perhaps we'll toggle the pref to disable these properties before running the mochitest, or something.)

I imagine we probably want to remove the unprefixed properties from part 4 before landing, too ("text-size-adjust", "appearance", "user-select"). We can always add them back later if web-compat demands it; but for now, it's probably simplest to only concern ourselves with -webkit prefixed properties here.

Note: I needed to do a bit more rebasing locally this morning in part 1, to fix bitrot from bug 1206569 [which, among other things, renamed CSS2PropertiesProps.h to PythonCSSProps.h]. I won't bother to repost just for that bitrot-fix at this point. Also: the current patch stack hit a silly test-failure in a preliminary Try run that I kicked off last night[1], and I filed bug 1208136 to fix that. [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0e85628c5ce

Here's a green try run, with these patches layered on top of my patch in bug 1208136, FWIW (on one platform): https://treeherder.mozilla.org/#/jobs?repo=try&revision=4675d846127b

(In reply to Daniel Holbert [:dholbert] from comment #26) > Note: for simplicity, I'm keeping the preference name > ("layout.css.test-webkit-prefixes") unchanged for now (I'm tentatively planning on renaming this to "layout.css.webkit-prefixed-aliases.enabled" -- that's what I'm using locally.)

Two more small changes I've made locally in my interdiff for part 4: (1) I'm naming the pref "layout.css.prefixes.webkit" for this feature now, since that's how we've been naming preferences for prefixed css features: http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js?rev=f33b94206b39#2292 (2) I'll now be adding this pref to libpref/init/all.js , set to false by default, instead of making it a "hidden" pref. (Otherwise we'll fail tests -- intentionally, as of the patch for bug 1208136.) (I won't bother re-requesting review for these changes, though if anyone objects to this pref name, feel free to speak up; and we can always adjust it later on, too.)

dbaron: just to be on the safe side, could you confirm that you're OK with me pushing your unbitrotted patches here? (with my interdiff tweaks folded in) I'm confident that they're landable, particularly given that this will be preffed off by default for now; but I want to make sure I've got your approval to land them.

I guess I don't think there's any need for patches 2 and 3 anymore. (They also somewhat conflict with the work in bug 1069192, which adds a different mechanism to do roughly the same thing.) And probably not patch 5 either, actually.

(In reply to Daniel Holbert [:dholbert] from comment #32) > Two more small changes I've made locally in my interdiff for part 4: > (1) I'm naming the pref "layout.css.prefixes.webkit" for this feature now, > since that's how we've been naming preferences for prefixed css features: > http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all. > js?rev=f33b94206b39#2292 > > (2) I'll now be adding this pref to libpref/init/all.js , set to false by > default, instead of making it a "hidden" pref. (Otherwise we'll fail tests > -- intentionally, as of the patch for bug 1208136.) Those both sound fine.

(In reply to David Baron [:dbaron] ⌚UTC-7 from comment #34) > I guess I don't think there's any need for patches 2 and 3 anymore. (They > also somewhat conflict with the work in bug 1069192, which adds a different > mechanism to do roughly the same thing.) OK, thanks -- that makes sense. > And probably not patch 5 either, actually. Sure, since we're planning on enabling these before too long, no sense in adding a test that asserts they're disabled. Sounds good. I'll plan on landing part 4 on its own, then -- with the interdiff folded in -- after a local sanity-check to make sure it works as expected & a run through Try.

Green try run, with this bug's tweaked 'part 4' patch & bug 1211101's mochitests for these properties (property_database.js additions): https://treeherder.mozilla.org/#/jobs?repo=try&revision=54d02f56d9e2

[updating summary to reflect the final intent of this bug (this is no longer just for debugging site problems)]

I've added compatibility notes to the pages below: https://developer.mozilla.org/en-US/docs/Web/CSS/animation https://developer.mozilla.org/en-US/docs/Web/CSS/animation-delay https://developer.mozilla.org/en-US/docs/Web/CSS/animation-direction https://developer.mozilla.org/en-US/docs/Web/CSS/animation-duration https://developer.mozilla.org/en-US/docs/Web/CSS/animation-fill-mode https://developer.mozilla.org/en-US/docs/Web/CSS/animation-iteration-count https://developer.mozilla.org/en-US/docs/Web/CSS/animation-name https://developer.mozilla.org/en-US/docs/Web/CSS/animation-play-state https://developer.mozilla.org/en-US/docs/Web/CSS/animation-timing-function https://developer.mozilla.org/en-US/docs/Web/CSS/text-size-adjust https://developer.mozilla.org/en-US/docs/Web/CSS/transform https://developer.mozilla.org/en-US/docs/Web/CSS/transform-origin https://developer.mozilla.org/en-US/docs/Web/CSS/transform-style https://developer.mozilla.org/en-US/docs/Web/CSS/transition https://developer.mozilla.org/en-US/docs/Web/CSS/transition-delay https://developer.mozilla.org/en-US/docs/Web/CSS/transition-duration https://developer.mozilla.org/en-US/docs/Web/CSS/transition-property https://developer.mozilla.org/en-US/docs/Web/CSS/transition-timing-function https://developer.mozilla.org/en-US/docs/Web/CSS/border-radius https://developer.mozilla.org/en-US/docs/Web/CSS/border-top-left-radius https://developer.mozilla.org/en-US/docs/Web/CSS/border-top-right-radius https://developer.mozilla.org/en-US/docs/Web/CSS/border-bottom-left-radius https://developer.mozilla.org/en-US/docs/Web/CSS/border-bottom-right-radius https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-appearance https://developer.mozilla.org/en-US/docs/Web/CSS/background-clip https://developer.mozilla.org/en-US/docs/Web/CSS/background-origin https://developer.mozilla.org/en-US/docs/Web/CSS/background-size https://developer.mozilla.org/en-US/docs/Web/CSS/border-image https://developer.mozilla.org/en-US/docs/Web/CSS/box-shadow https://developer.mozilla.org/en-US/docs/Web/CSS/box-sizing https://developer.mozilla.org/en-US/docs/Web/CSS/user-select And I added a release note to 44: https://developer.mozilla.org/en-US/Firefox/Releases/44 David, can you please have a look whether that's ok? Sebastian

Daniel, as David seems to be quite busy, could you please help and quickly review the pages I listed in comment 45? Sebastian

Sorry, I'm quite busy up as well (in part because I might be out of the office for some or all of next week to serve on a jury, depending on whether I'm called in or not -- and I've got a backlog of other work & requests to get through before then). (I'll leave this in my needinfo queue, but I might not get to it right away.)

Fine for me. Take your time. I just wanted to make sure it's not forgotten. (And getting a "I'm busy" answer is better than no answer. :-) ) Sebastian

> (In reply to Sebastian Zartner [:sebo] from comment #45) > I've added compatibility notes to the pages below: ... > https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-appearance .... > It looks like you're just documenting the aliases added by this patch (I see some of the ones added later like -webkit-filter are also documented) -- everything looks good except -moz-appearance. That was removed in Bug 1249134.

Sorry for the delay! (In reply to Mike Taylor [:miketaylr] from comment #49) > > (In reply to Sebastian Zartner [:sebo] from comment #45) > > I've added compatibility notes to the pages below: > ... > > https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-appearance > .... > > > > It looks like you're just documenting the aliases added by this patch (I see > some of the ones added later like -webkit-filter are also documented) Yes, documentation for other aliases was done separately for each of the bugs scattered over Bugzilla (-webkit-filter was implemented in bug 1236506). Unfortunately not all aliases are referenced correctly and implementers tend to forget to set the dev-doc-needed flag, which made them a bit hard to find. So, if you see -webkit alias bugs I've missed to document, please add the dev-doc-needed flag, so their documentation can be tracked. > everything looks good except -moz-appearance. That was removed in Bug > 1249134. Thank you for the info! I've removed the note from -moz-appearance again. With that done I set this to dev-doc-complete now. Sebastian