In "-webkit-linear-gradient" fixup 3-part fix, each part stomps on previous part

[dholbert]

The "-webkit-linear-gradient" fixup code has this right now:

   newValue = value.replace(/-webkit-/, '');
   newValue = value.replace(/(top|bottom)/, 'to $1');
   newValue = value.replace(/\d+deg/, function (val) {

https://github.com/hallvors/css-fixme/blob/master/cssfixme.htm#L330

This seems to be a bug. The first line strips "-webkit" from |value|, and stores that in |newValue|. But then in the next line, we stomp on |newValue| with a different |value|-derived thing. (The variable |value| remains unchanged, so it's still got "-webkit-" in it.)

So the third quoted line here is really the only line that has an effect. The first two get undone.

I think this wants to say:

newValue = value; newValue = newValue.replace(...); newValue = newValue.replace(...); newValue = newValue.replace(...);

[hallvors]

Ouch - that's indeed a very silly looking bug :-/ How did I never catch that? :-p

On Wed, Mar 25, 2015 at 10:05 PM, Daniel Holbert notifications@github.com wrote:

The "-webkit-linear-gradient" fixup code has this right now:

newValue = value.replace(/-webkit-/, ''); newValue = value.replace(/(top|bottom)/, 'to $1'); newValue = value.replace(/\d+deg/, function (val) {

https://github.com/hallvors/css-fixme/blob/master/cssfixme.htm#L330

This seems to be a bug. The first line strips "-webkit" from |value|, and stores that in |newValue|. But then in the next line, we stomp on |newValue| with a different |value|-derived thing. (The variable |value| remains unchanged, so it's still got "-webkit-" in it.)

So the third quoted line here is really the only line that has an effect. The first two get undone.

I think this wants to say:

newValue = value; newValue = newValue.replace(...); newValue = newValue.replace(...); newValue = newValue.replace(...);

— Reply to this email directly or view it on GitHub https://github.com/hallvors/css-fixme/issues/4.

[dholbert]

(FWIW, I'm partly refactoring this code, to better-serve my purposes in https://bugzilla.mozilla.org/show_bug.cgi?id=1132748 -- I'll tag you for review or feedback over there.

If you like the refactored version, you may want to end up copying its tweaks back over here, to improve maintainability & improve ability to continue sharing code between this script & firefox's now-builtin unprefixing service going forward.)

[hallvors]

This code obviously needs some review and care :-p - so thanks a lot, I'll follow your changes with interest..

[hallvors]

Fix: https://github.com/hallvors/css-fixme/commit/90c002b76e3b0a23bed05122fdf91d974417d009

A bug earlier in the callstack meant this code wasn't really running when you might expect it to, which might again explain why it was so buggy and untested...