2023648 - The var() function seems to be broken when it spans several lines

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

Attachment: testcase

STR:

1. Open the attachment.
2. Inspect the cyan square.

=> Notice that the var name is absent in the rule.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: screenshot

Comment 2

[Julien Wajsberg [:julienw]]

The source is:

div {
  width: var(
    --foo, 500px
  );
  height: 500px;
  background: cyan;
}

But the result in the devtools' inspector is:

div {
  width: var(, 500px);
  height: 500px;
  background: cyan;
}

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 2023648 - [devtools] Fix OutputParser handling of var() with whitespaces. r=#devtools

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

submitted patch to fix the issue. This could also be triggered with something as simple as having a space before the var name (e.g. var( --x))

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1854104

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/af634291e68f
https://hg.mozilla.org/integration/autoland/rev/0c0a60e4cf37
[devtools] Fix OutputParser handling of var() with whitespaces. r=devtools-reviewers,bomsy

Comment 7

[amarc]

https://hg.mozilla.org/mozilla-central/rev/0c0a60e4cf37

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

(asked for beta uplift, even though I know there won't be any beta release, I just want the patch to be included in 149 so we don't ship the bug in release)

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 2023648 - [devtools] Fix OutputParser handling of var() with whitespaces.

In Bug 1854104 I made a change to in #parseMatchingParens to return all the
tokens we parsed, including whitespaces.
This threw off the logic for parsing var(), so we're reverting this.
Because of this, we need a small adjustment in the function responsible to
parse attr() (adding a space after we handle a Comma).

Original Revision: https://phabricator.services.mozilla.com/D288689

Comment 10

[Phabricator Automation]

firefox-release Uplift Approval Request

• User impact if declined/Reason for urgency: Users won't see variables used in var() in the inspector if there's whitespace before the variable name
• Code covered by automated testing?: yes
• Fix verified in Nightly?: no
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing: -
• Risk associated with taking this patch: low
• Explanation of risk level: simple devtools-only change, reverting the regressing patch, covered by test
• String changes made/needed?: -
• Is Android affected?: no

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 2023648 - [devtools] Fix OutputParser handling of var() with whitespaces.

In Bug 1854104 I made a change to in #parseMatchingParens to return all the
tokens we parsed, including whitespaces.
This threw off the logic for parsing var(), so we're reverting this.
Because of this, we need a small adjustment in the function responsible to
parse attr() (adding a space after we handle a Comma).

Original Revision: https://phabricator.services.mozilla.com/D288689

Comment 12

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/ab4ef1478ff0
https://hg.mozilla.org/releases/mozilla-release/rev/4876d6b90743