1712389 - Custom Properties that begin with numbers show as invalid

Status: RESOLVED FIXED

(DevTools :: General, defect, P2)

Description

[Anthony Frehner]

Attachment: An example of a correct Custom Property that's marked as "invalid"

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.2 Safari/605.1.15

Steps to reproduce:

Set a custom property that begins with a number.

For example, --100: 100px;

DevTools views that as invalid, and strikes-through the property. Yet the property still actually works correctly, which leads me to believe this is just a DevTools issue alone.

Additionally, running Javascript to set a Custom Property causes that property to never show up in DevTools anywhere. However, it too is still valid and renders correctly.

For example, doing the following in a script:

document.documentElement.style.setProperty('--100', '100px')

Means that the Custom Property never shows up anywhere in DevTools.

Additionally, if you try to update a Custom Property by running the above command in the DevTools console, it does not update the Custom Property. For example, assuming the Custom Property has already been set to '100px' as above:

(In the DevTools console)
document.documentElement.style.setProperty('--100', '20px')

The Custom Property remains at '100px'

Actual results:

Custom Properties that begin with numbers either appear invalid, don't show up at all, or aren't updated when executing Javascript in the DevTools console.

Expected results:

Custom Properties that begin with numbers should be valid in DevTools.

Custom Properties that begin with numbers, and are set by Javascript should appear and be valid.

Custom Properties that begin with numbers, and that are set/updated by Javascript in the DevTools console should appear and be updated on the fly.

Comment 1

[Julian Descottes [:jdescottes]]

I think we misunderstood the spec when defining our regexp to validate CSS variables.

The regexp is at https://searchfox.org/mozilla-central/rev/0adc9c8175bcc7cbc1da6adbfe40f9fe18836ac5/devtools/client/fronts/css-properties.js#40-45

var FIRST_CHAR = ["[_a-z]", NON_ASCII, ESCAPE].join("|");
var TRAILING_CHAR = ["[_a-z0-9-]", NON_ASCII, ESCAPE].join("|");
var IS_VARIABLE_TOKEN = new RegExp(
  `^--(${FIRST_CHAR})(${TRAILING_CHAR})*$`,
  "i"
);

Which means we explicitly forbid 0-9 as starting char.

However the spec says that a valid custom property name is "any valid identifier that starts with two dashes". Identifier in this context is an ident-token, which should follow the regex at: https://www.w3.org/TR/css-syntax-3/#ident-token-diagram

If I'm reading this correctly, it means that an ident-token which starts with -- can immediately use digits.

I think we should just update the regexp I linked above, and add a test similar to devtools/client/inspector/rules/test/browser_rules_variables_02.js

This should be a good first bug.

Comment 2

[Anthony Frehner]

That is the exact same conclusion I came to as well. Agreed 100% with your assessment.

I can't promise anything, but I haven't contributed before and I have some interest in attempting to fix this. If I do, I'll get to it this weekend, otherwise you can assume that I didn't succeed or that I couldn't get to it.

If that's ok?

Comment 3

[Julian Descottes [:jdescottes]]

(In reply to afrehner.work from comment #2)

That is the exact same conclusion I came to as well. Agreed 100% with your assessment.

I can't promise anything, but I haven't contributed before and I have some interest in attempting to fix this. If I do, I'll get to it this weekend, otherwise you can assume that I didn't succeed or that I couldn't get to it.

If that's ok?

Sure, thanks for proposing to work on this (and thanks for filing the bug in the first place :) ) !
You can take a look at the contributor's doc at https://firefox-source-docs.mozilla.org/setup/index.html

If you have issues/questions don't hesitate to ask here. You can use the "Request information from" below the comment box to ping someone directly, best way to make sure we don't forget your question. Or you can join https://chat.mozilla.org/#/room/#devtools:mozilla.org and ask questions there if you prefer.

Maybe try to first fix the regex, and we can work on tests in a second step? I'll assign the bug to you for now.
(and no worries if you can't get to it, we'll reassign it in case there's no activity for a few weeks)

Comment 4

[Anthony Frehner]

Attachment: Bug 1712389 - Update devtools to allow valid CSS variables. r?jdescottes

Comment 5

[Julian Descottes [:jdescottes]]

ni myself to check for try results and land the patch

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d1b785f4508
Update devtools to allow valid CSS variables. r=jdescottes

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/7d1b785f4508