Bug 154215 - Web Inspector: Show inherited CSS variables in the Style sidebar
Summary: Web Inspector: Show inherited CSS variables in the Style sidebar
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on:
Blocks: 154082 154216 154217
  Show dependency treegraph
 
Reported: 2016-02-13 09:54 PST by Timothy Hatcher
Modified: 2016-02-15 13:47 PST (History)
9 users (show)

See Also:


Attachments
Patch (3.36 KB, patch)
2016-02-13 10:19 PST, Timothy Hatcher
joepeck: review+
joepeck: commit-queue-
Details | Formatted Diff | Diff
Screenshot (252.13 KB, image/png)
2016-02-13 10:20 PST, Timothy Hatcher
no flags Details
Computed Styles Screenshot (133.62 KB, image/png)
2016-02-13 10:43 PST, Timothy Hatcher
no flags Details
Patch for Landing (4.12 KB, patch)
2016-02-15 13:23 PST, Timothy Hatcher
timothy: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2016-02-13 09:54:32 PST
When a selector only has CSS variables and no other inherited or applied properties, we don't show the rule.
Comment 1 Radar WebKit Bug Importer 2016-02-13 09:54:42 PST
<rdar://problem/24644058>
Comment 2 Timothy Hatcher 2016-02-13 10:19:41 PST
Created attachment 271284 [details]
Patch
Comment 3 Timothy Hatcher 2016-02-13 10:20:12 PST
Created attachment 271285 [details]
Screenshot
Comment 4 Timothy Hatcher 2016-02-13 10:23:07 PST
Some follow up bugs. Bug 154216 and bug 154217.
Comment 5 Timothy Hatcher 2016-02-13 10:43:17 PST
Created attachment 271291 [details]
Computed Styles Screenshot

They show up in computed styles, as expected too.
Comment 6 Nikita Vasilyev 2016-02-15 00:06:03 PST
Comment on attachment 271284 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=271284&action=review

Looks good to me, but I'm not a reviewer.

> Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:45
> +        return /^--[\w-]+$/.test(name);

"--9" is a valid CSS variable name in WebKit, I just checked. This regex should be fine.
Comment 7 Devin Rousso 2016-02-15 03:30:45 PST
(In reply to comment #6)
> Comment on attachment 271284 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=271284&action=review
> 
> Looks good to me, but I'm not a reviewer.
> 
> > Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:45
> > +        return /^--[\w-]+$/.test(name);
> 
> "--9" is a valid CSS variable name in WebKit, I just checked. This regex
> should be fine.

So I just tested this, and the variable name "--" is valid in the Nightlies.  Not really sure if that's a bug, as it doesn't work in Chrome, but if that is allowed via the spec then we may want to change the regex to /^--[\w-]*$/
Comment 8 Joseph Pecoraro 2016-02-15 12:44:16 PST
(In reply to comment #7)
> (In reply to comment #6)
> > Comment on attachment 271284 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=271284&action=review
> > 
> > Looks good to me, but I'm not a reviewer.
> > 
> > > Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:45
> > > +        return /^--[\w-]+$/.test(name);
> > 
> > "--9" is a valid CSS variable name in WebKit, I just checked. This regex
> > should be fine.
> 
> So I just tested this, and the variable name "--" is valid in the Nightlies.
> Not really sure if that's a bug, as it doesn't work in Chrome, but if that
> is allowed via the spec then we may want to change the regex to /^--[\w-]*$/

A custom property is defined as:
<https://drafts.csswg.org/css-variables/#defining-variables> (CSS Custom Properties 1)
> A custom property is any property whose name starts with two dashes
> (U+002D HYPHEN-MINUS), like --foo. The <custom-property-name>
> production corresponds to this: it’s defined as any valid identifier
> that starts with two dashes.

An "identifier" here means:
<https://drafts.csswg.org/css-syntax-3/#identifier> (CSS Syntax 3)
> identifier
>   A portion of the CSS source that has the same syntax as an <ident-token>.

And an <ident-token> has this sweet diagram:
<https://drafts.csswg.org/css-syntax-3/#ident-token-diagram>

So if we are looking at <ident-token> starting with two dashes (a subset of that diagram), then yes, I see both "--" and "--9" as valid tokens.
Comment 9 Joseph Pecoraro 2016-02-15 12:48:59 PST
Comment on attachment 271284 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=271284&action=review

r=me

>>>> Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:45
>>>> +        return /^--[\w-]+$/.test(name);
>>> 
>>> "--9" is a valid CSS variable name in WebKit, I just checked. This regex should be fine.
>> 
>> So I just tested this, and the variable name "--" is valid in the Nightlies.  Not really sure if that's a bug, as it doesn't work in Chrome, but if that is allowed via the spec then we may want to change the regex to /^--[\w-]*$/
> 
> A custom property is defined as:
> <https://drafts.csswg.org/css-variables/#defining-variables> (CSS Custom Properties 1)

Technically an escape sequence is allowed here as well:
<https://drafts.csswg.org/css-syntax-3/#escape-diagram>

    --\u1234: valid;

Perhaps just checking if it starts with double dashes is enough?

    name.startsWith("--");
Comment 10 Timothy Hatcher 2016-02-15 13:23:45 PST
Created attachment 271365 [details]
Patch for Landing
Comment 11 WebKit Commit Bot 2016-02-15 13:37:09 PST
Comment on attachment 271365 [details]
Patch for Landing

Clearing flags on attachment: 271365

Committed r196597: <http://trac.webkit.org/changeset/196597>
Comment 12 WebKit Commit Bot 2016-02-15 13:37:12 PST
All reviewed patches have been landed.  Closing bug.