1551816 - Rules panel blank if CSS variable contains url function with parenthesis

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect, P1)

Description

[Christian Sonne [:cers]]

Attachment: Simplified test case

If an element is targeted by a rule that includes a CSS variable declaration, and the value contains the url function with an unescaped start parenthesis inside, that element shows an empty rules panel in the inspector.

div {
  --somevar: url(());
}

Steps to reproduce

1. load attached test case
2. inspect elements on page
3. observe rules panel

Expected outcome

All elements should display the relevant CSS applied to them in the rules panel

Actual outcome

Elements marked with green background work fine, but those marked with red have an empty rules panel.

Notes

Children appear to be unaffected.

Comment 1

[Christian Sonne [:cers]]

mozregression narrowed it down to https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=87824406b9feb420a3150720707b424d7cee5915&tochange=51ffb9283f0c7c00e08eb8c39b33fbee218c370d

and my best guess is that the changes in bug 1383075 is causing this.

Comment 2

[Patrick Brosset <:pbro>]

Thank you for filing.
I'm seeing the following stack trace in the browser console:

TypeError: "/^(url\([ \t\r\n\f]*(["']?))(.*?)(\2[ \t\r\n\f]*\))$/i.exec(...) is null"
    _appendURL resource://devtools/client/shared/output-parser.js:1344
    _doParse resource://devtools/client/shared/output-parser.js:424
    _parse resource://devtools/client/shared/output-parser.js:507
    parseCssProperty resource://devtools/client/shared/output-parser.js:102
    update resource://devtools/client/inspector/rules/views/text-property-editor.js:414
    TextPropertyEditor resource://devtools/client/inspector/rules/views/text-property-editor.js:101
    populate resource://devtools/client/inspector/rules/views/rule-editor.js:470
    _create resource://devtools/client/inspector/rules/views/rule-editor.js:216
    RuleEditor resource://devtools/client/inspector/rules/views/rule-editor.js:79
    _createEditors resource://devtools/client/inspector/rules/rules.js:1167
    _populate resource://devtools/client/inspector/rules/rules.js:973
    promise callback*_populate resource://devtools/client/inspector/rules/rules.js:967
    selectElement resource://devtools/client/inspector/rules/rules.js:876
    process resource://gre/modules/Promise-backend.js:923
    walkerLoop resource://gre/modules/Promise-backend.js:807
    scheduleWalkerLoop resource://gre/modules/Promise-backend.js:740
    schedulePromise resource://gre/modules/Promise-backend.js:771
    then resource://gre/modules/Promise-backend.js:456
    selectElement resource://devtools/client/inspector/rules/rules.js:860
    onSelected resource://devtools/client/inspector/rules/rules.js:1935

Comment 3

[Patrick Brosset <:pbro>]

We use a regex here to break down the argument of a url() function, but the code just assumes the regex would always match. Because it doesn't here, an exception is raised and causes the rule-view to be blank.
We should handle this case too.

Setting as a P1 since this is a crash bug.

Comment 4

[Patrick Brosset <:pbro>]

Attachment: Bug 1551816 - Bail out rendering urls in rule-view if they're not valid; r?gl!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/72820b3eb2f0
Bail out rendering urls in rule-view if they're not valid; r=gl

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/72820b3eb2f0