Unprefix CSS Variables

Unprefix CSS Variables

CSS Variables still have the -webkit prefix.
This CL removes the prefix.
Old format: -webkit-var-name: -webkit-var(otherName)
New format: var-name: var(otherName);

New test: fast/css/variables/prefix.html

BUG=244352
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=152747

[Mike Lawther (Google), 2013-05-29 04:54:13]

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp
File Source/core/css/CSSParser.cpp (right):

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp...
Source/core/css/CSSParser.cpp:9799: if
(RuntimeEnabledFeatures::cssVariablesEnabled() &&
isASCIIAlphaCaselessEqual(name[0], 'v') && isASCIIAlphaCaselessEqual(name[1],
'a') && isASCIIAlphaCaselessEqual(name[2], 'r')) {
This is a job for a new patch, but this looks like it would work with eg "VaR"?
The spec text currently says: "The "var-" prefix must be written in
lower-case.".

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp...
Source/core/css/CSSParser.cpp:10255: if (name[3] == '-' &&
isIdentifierStartAfterDash(name + 4) && isEqualToCSSIdentifier(name, "var"))
You can get rid of these magic numbers with something like
static const char* token = "var";
const unsigned tokenLength = sizeof(token) - 1;
const unsigned prefixLength = tokenLength + (sizeof("-") - 1);

Now you can replace '3' with 'tokenLength', and replace '4' with 'prefixLength'.

[alancutter (OOO until 2018), 2013-05-29 06:41:51]

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp
File Source/core/css/CSSParser.cpp (right):

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp...
Source/core/css/CSSParser.cpp:9799: if
(RuntimeEnabledFeatures::cssVariablesEnabled() &&
isASCIIAlphaCaselessEqual(name[0], 'v') && isASCIIAlphaCaselessEqual(name[1],
'a') && isASCIIAlphaCaselessEqual(name[2], 'r')) {
On 2013/05/29 04:54:13, Mike Lawther (Google) wrote:
> This is a job for a new patch, but this looks like it would work with eg
"VaR"?
> The spec text currently says: "The "var-" prefix must be written in
> lower-case.".

I was unsure whether I should tackle that in this patch or not. I decided it
would be prudent to keep the case sensitivity of var as a separate patch.

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp...
Source/core/css/CSSParser.cpp:10255: if (name[3] == '-' &&
isIdentifierStartAfterDash(name + 4) && isEqualToCSSIdentifier(name, "var"))
On 2013/05/29 04:54:13, Mike Lawther (Google) wrote:
> You can get rid of these magic numbers with something like
> static const char* token = "var";
> const unsigned tokenLength = sizeof(token) - 1;
> const unsigned prefixLength = tokenLength + (sizeof("-") - 1);
> 
> Now you can replace '3' with 'tokenLength', and replace '4' with
'prefixLength'.

I would prefer to have the entire "var-" as a string for searchability.
The token is really the entire thing rather than just the "var" bit. It's hard
to label that as anything other than prefixWithoutDash.
Would it be okay to just have:

ASSERT(prefixLength > 0)
if (name[prefixLength - 1] == '-' && ...

?

[Mike Lawther (Google), 2013-05-29 08:23:00]

lgtm

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp
File Source/core/css/CSSParser.cpp (right):

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp...
Source/core/css/CSSParser.cpp:9799: if
(RuntimeEnabledFeatures::cssVariablesEnabled() &&
isASCIIAlphaCaselessEqual(name[0], 'v') && isASCIIAlphaCaselessEqual(name[1],
'a') && isASCIIAlphaCaselessEqual(name[2], 'r')) {
On 2013/05/29 06:41:51, alancutter wrote:
> On 2013/05/29 04:54:13, Mike Lawther (Google) wrote:
> > This is a job for a new patch, but this looks like it would work with eg
> "VaR"?
> > The spec text currently says: "The "var-" prefix must be written in
> > lower-case.".
> 
> I was unsure whether I should tackle that in this patch or not. I decided it
> would be prudent to keep the case sensitivity of var as a separate patch.

I agree - post a new patch for this.

https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp...
Source/core/css/CSSParser.cpp:10255: if (name[3] == '-' &&
isIdentifierStartAfterDash(name + 4) && isEqualToCSSIdentifier(name, "var"))
On 2013/05/29 06:41:51, alancutter wrote:
> On 2013/05/29 04:54:13, Mike Lawther (Google) wrote:
> > You can get rid of these magic numbers with something like
> > static const char* token = "var";
> > const unsigned tokenLength = sizeof(token) - 1;
> > const unsigned prefixLength = tokenLength + (sizeof("-") - 1);
> > 
> > Now you can replace '3' with 'tokenLength', and replace '4' with
> 'prefixLength'.
> 
> I would prefer to have the entire "var-" as a string for searchability.
> The token is really the entire thing rather than just the "var" bit. It's hard
> to label that as anything other than prefixWithoutDash.
> Would it be okay to just have:
> 
> ASSERT(prefixLength > 0)
> if (name[prefixLength - 1] == '-' && ...
> 
> ?

Can you make that a COMPILE_ASSERT? No need to have a runtime check for what is
a compile time problem :)

[alancutter (OOO until 2018), 2013-05-30 06:59:38]

On 2013/05/29 08:23:00, Mike Lawther (Google) wrote:
> lgtm
> 
> https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp
> File Source/core/css/CSSParser.cpp (right):
> 
>
https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp...
> Source/core/css/CSSParser.cpp:9799: if
> (RuntimeEnabledFeatures::cssVariablesEnabled() &&
> isASCIIAlphaCaselessEqual(name[0], 'v') && isASCIIAlphaCaselessEqual(name[1],
> 'a') && isASCIIAlphaCaselessEqual(name[2], 'r')) {
> On 2013/05/29 06:41:51, alancutter wrote:
> > On 2013/05/29 04:54:13, Mike Lawther (Google) wrote:
> > > This is a job for a new patch, but this looks like it would work with eg
> > "VaR"?
> > > The spec text currently says: "The "var-" prefix must be written in
> > > lower-case.".
> > 
> > I was unsure whether I should tackle that in this patch or not. I decided it
> > would be prudent to keep the case sensitivity of var as a separate patch.
> 
> I agree - post a new patch for this.

I've been beaten to the punch!
https://codereview.chromium.org/16081003

>
https://codereview.chromium.org/16183002/diff/1/Source/core/css/CSSParser.cpp...
> Source/core/css/CSSParser.cpp:10255: if (name[3] == '-' &&
> isIdentifierStartAfterDash(name + 4) && isEqualToCSSIdentifier(name, "var"))
> On 2013/05/29 06:41:51, alancutter wrote:
> > On 2013/05/29 04:54:13, Mike Lawther (Google) wrote:
> > > You can get rid of these magic numbers with something like
> > > static const char* token = "var";
> > > const unsigned tokenLength = sizeof(token) - 1;
> > > const unsigned prefixLength = tokenLength + (sizeof("-") - 1);
> > > 
> > > Now you can replace '3' with 'tokenLength', and replace '4' with
> > 'prefixLength'.
> > 
> > I would prefer to have the entire "var-" as a string for searchability.
> > The token is really the entire thing rather than just the "var" bit. It's
hard
> > to label that as anything other than prefixWithoutDash.
> > Would it be okay to just have:
> > 
> > ASSERT(prefixLength > 0)
> > if (name[prefixLength - 1] == '-' && ...
> > 
> > ?
> 
> Can you make that a COMPILE_ASSERT? No need to have a runtime check for what
is
> a compile time problem :)

Done.

[alancutter (OOO until 2018), 2013-06-14 05:01:28]

Rebased to ToT.

[Mike Lawther (Google), 2013-06-14 07:08:09]

This needs lgtm from an owner - anyone on the CC list who could take a look
please do!

[Hajime Morrita, 2013-06-20 04:12:23]

subbetstamping lgtm.

[commit-bot: I haz the power, 2013-06-20 04:12:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alancutter@chromium.org/16183002/43001

[commit-bot: I haz the power, 2013-06-20 04:43:26]

Change committed as 152747