Bug 99259 - Web Inspector: Sass can only resolve same folder paths
Summary: Web Inspector: Sass can only resolve same folder paths
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-13 21:44 PDT by Paul Irish
Modified: 2012-10-23 03:05 PDT (History)
10 users (show)

See Also:


Attachments
Patch (11.30 KB, patch)
2012-10-22 05:57 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
Patch (12.08 KB, patch)
2012-10-23 01:17 PDT, Alexander Pavlov (apavlov)
vsevik: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Irish 2012-10-13 21:44:43 PDT
A few ways to trigger this issue

Use a folder structure like 
|- css\*
\- sass\*

Or one like
\-css\*
  \-sass\*

1) Compile sass with line numbers.

2) above a block of styles, view the filename
2a) Bug: it may appear as "css/..:213" which doesn't give the source filename

3) click the file link

4a) on the file:/// protocol, a new tab opens pointing to an Index page for the folder that contains the host page.
4b) with the project in localhost, clicking the filename link opens the host page as (program) in sources, but jumps to line 213
Comment 1 Alexander Pavlov (apavlov) 2012-10-15 08:08:41 PDT
@Paul: you are using the "Support for SASS" experiment, right?

@Seva: can you comment on the bug once the resources start appearing for the *.scss files (per our offline discussion)?
Comment 2 Alexander Pavlov (apavlov) 2012-10-22 05:39:18 PDT
(In reply to comment #0)
> 2) above a block of styles, view the filename
> 2a) Bug: it may appear as "css/..:213" which doesn't give the source filename

Could not reproduce this on tip of tree. I get "css/test.scss:213", which is ugly, of course, and I'm going to address this.

> 3) click the file link
> 
> 4a) on the file:/// protocol, a new tab opens pointing to an Index page for the folder that contains the host page.

Could not reproduce this either. With the experiment enabled, the Sources panel navigates to the correct line in the original scss.

> 4b) with the project in localhost, clicking the filename link opens the host page as (program) in sources, but jumps to line 213

Not sure what the correct behavior should be in this case, since the debug info generated by SASS provides absolute file://... URLs for the rules' original locations in scss files. This might or might not work at all.
Comment 3 Alexander Pavlov (apavlov) 2012-10-22 05:57:36 PDT
Created attachment 169889 [details]
Patch
Comment 4 Vsevolod Vlasov 2012-10-23 00:18:47 PDT
Comment on attachment 169889 [details]
Patch

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

> Source/WebCore/inspector/front-end/CSSStyleModel.js:505
> +        if (!location.uiLocation())

Does it ever happen? We should avoid having rawLocation that does not have matching uiLocation as much as possible.

> Source/WebCore/inspector/front-end/CSSStyleModel.js:508
> +            this._locations = [];

nit: We will always have some live locations on CSSStyleModel, so I would move this._locations = []; to constructor (and annotate it) to make code simpler.

> Source/WebCore/inspector/front-end/Linkifier.js:113
> +    _updateCSSAnchor: function(anchor, uiLocation)

You should reuse _updateAnchor and move CSS domain specific logic to you own LinkifierFormatter implementation (probably inherited form DefaultFormatter).
Comment 5 Alexander Pavlov (apavlov) 2012-10-23 01:17:16 PDT
Created attachment 170076 [details]
Patch
Comment 6 Vsevolod Vlasov 2012-10-23 02:19:39 PDT
Comment on attachment 170076 [details]
Patch

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

> Source/WebCore/inspector/front-end/CSSStyleModel.js:516
> +        if (!this._locations)

Redundant.

> Source/WebCore/inspector/front-end/CSSStyleModel.js:823
> +WebInspector.CSSRule.Location = function(rawLocation, updateDelegate)

Where is nothing rule specific here, let's call it WebInspector.CSSStyleModel.Location instead?
Comment 7 Alexander Pavlov (apavlov) 2012-10-23 03:05:47 PDT
Committed r132197: <http://trac.webkit.org/changeset/132197>