Bug 174329

Summary: Web Inspector: [PARITY] Styles: Command-click on a property name should jump to definition in Resources tab
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 145982    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
[Animated GIF] With patch applied none

Description Nikita Vasilyev 2017-07-10 16:33:00 PDT
In Rules panel, clicking on a CSS property while holding Command key, should navigate to corresponding line number in Resources tab.
Currently, we use Option-click for that. Command-click inserts a second text caret (implemented by CodeMirror).
Comment 1 Radar WebKit Bug Importer 2017-07-10 16:33:26 PDT
<rdar://problem/33225564>
Comment 2 Nikita Vasilyev 2017-10-13 22:34:44 PDT
Created attachment 323794 [details]
Patch
Comment 3 Nikita Vasilyev 2017-10-13 22:42:36 PDT
Created attachment 323795 [details]
[Animated GIF] With patch applied
Comment 4 Joseph Pecoraro 2017-10-15 15:15:28 PDT
Comment on attachment 323794 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:265
> +            let range = this._property.styleSheetTextRange;
> +            const options = {
> +                ignoreNetworkTab: true,
> +                ignoreSearchTab: true,
> +            };
> +            let sourceCode = sourceCodeLocation.sourceCode;
> +            WI.showSourceCodeLocation(sourceCode.createSourceCodeLocation(range.startLine, range.startColumn), options);

This looks like it would always take you to the styleSheetTextRange's start. But what would be more useful would be the key/value you clicked on's location. This is most useful when using Sass/Less with SourceMaps. Individual properties within a rule might come from a Mixin, and using the location of the property itself will get your to that Mixin's definition but other normal properties would take you to the rule's definition.
Comment 5 Nikita Vasilyev 2017-10-15 15:29:47 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=323794&action=review

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:265
>> +            WI.showSourceCodeLocation(sourceCode.createSourceCodeLocation(range.startLine, range.startColumn), options);
> 
> This looks like it would always take you to the styleSheetTextRange's start. But what would be more useful would be the key/value you clicked on's location. This is most useful when using Sass/Less with SourceMaps. Individual properties within a rule might come from a Mixin, and using the location of the property itself will get your to that Mixin's definition but other normal properties would take you to the rule's definition.

This takes you to the property start (e.g. property name). This is what the current styles sidebar does, I pretty much copy/pasted the code from WI.CSSStyleDeclarationTextEditor.prototype.tokenTrackingControllerHighlightedRangeWasClicked.

I agree this can be improved. Currently, there's CSSProperty.prototype._styleSheetTextRange, but no range for property value, AFAIK. I'd prefer to work on it later in a follow up issue.
Comment 6 Joseph Pecoraro 2017-10-16 11:09:56 PDT
(In reply to Nikita Vasilyev from comment #5)
> This takes you to the property start (e.g. property name).

Ahh! I misread, I thought it was the rule's start. This should be good enough to get expected behavior.
Comment 7 Joseph Pecoraro 2017-10-16 11:11:33 PDT
Comment on attachment 323794 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Base/Main.js:1460
> +    if (metaKeyDidChange)
> +        document.body.classList.toggle("meta-key-pressed", this.modifierKeys.metaKey);

I wonder how this will affect style recalculations changing a global property like this. Lets keep an eye out for any possible stutters after this lands.
Comment 8 WebKit Commit Bot 2017-10-17 11:39:51 PDT
Comment on attachment 323794 [details]
Patch

Clearing flags on attachment: 323794

Committed r223561: <https://trac.webkit.org/changeset/223561>
Comment 9 WebKit Commit Bot 2017-10-17 11:39:52 PDT
All reviewed patches have been landed.  Closing bug.