Closed Bug 1460606 Opened 2 years ago Closed 2 years ago

Update browser_parsable_css.js to support classnames containing "--"

Categories

(DevTools :: Application Panel, enhancement)

61 Branch
enhancement
Not set

Tracking

(firefox62 fixed)

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file)

In the DevTools Application panel, we would like to start following BEM guidelines (see Bug 1459581). This means we can have classes such as module__component--empty.

Currently the test browser_parsable_css.js will mistakenly think this is a CSS property and will complain.

We could update the regexp at https://searchfox.org/mozilla-central/rev/eb6c5214a63e20a3fff455e92c876287a8d2e188/browser/base/content/test/static/browser_parsable_css.js#265 to allow developers to use classnames containing "--"
Comment on attachment 8974710 [details]
Bug 1460606 - browser_parsable_css should not flag classnames with -- as properties;

https://reviewboard.mozilla.org/r/243076/#review248896

Thanks!

::: browser/base/content/test/static/browser_parsable_css.js:267
(Diff revision 1)
>      // Note: CSSStyleRule.cssText always has double quotes around URLs even
>      //       when the original CSS file didn't.
>      let urls = rule.cssText.match(/url\("[^"]*"\)/g);
> -    let props = rule.cssText.match(/(var\()?(--[\w\-]+)/g);
> +    // Extract props by searching all "--" preceeded by "var(" or a non-word
> +    // character.
> +    let props = rule.cssText.match(/(var\(|[^\w])(--[\w\-]+)/g);

You can use `\W` instead of `[^\w]`. :-)

::: browser/base/content/test/static/browser_parsable_css.js:296
(Diff revision 1)
>        if (prop.startsWith("var(")) {
>          prop = prop.substring(4);
>          let prevValue = customPropsToReferencesMap.get(prop) || 0;
>          customPropsToReferencesMap.set(prop, prevValue + 1);
> -      } else if (!customPropsToReferencesMap.has(prop)) {
> +      } else {
> +        prop = prop.substring(1);

Nit: can you add a comment here about why we need to do this?
Attachment #8974710 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8974710 [details]
Bug 1460606 - browser_parsable_css should not flag classnames with -- as properties;

https://reviewboard.mozilla.org/r/243076/#review248896

Thank you for the super-fast review! Try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=1eb829e0e7af9c18f85cbd3adedefe85c56b007e

> You can use `\W` instead of `[^\w]`. :-)

Nice, thanks!!

> Nit: can you add a comment here about why we need to do this?

Added a comment.
Blocks: 1459581
See Also: 1459581
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6ee32ec8320b
browser_parsable_css should not flag classnames with -- as properties;r=Gijs
https://hg.mozilla.org/mozilla-central/rev/6ee32ec8320b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.