Bug 150009 – Web Inspector: Line error widget showed in the wrong resource

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

Created attachment 262846 [details]
[Image] Bug

Steps:
1. Open http://nv.github.io/webkit-inspector-bugs/React-JSXTransformer/
2. Open Resources tab
3. Select the main HTML resource

Actual:
JS error "Can't find variable: nope" showed in the wrong (HTML) file.

Expected:
The error should only be showed in the JS file.

[Radar WebKit Bug Importer]

<rdar://problem/23062199>

[Joseph Pecoraro]

Is this the same as bug 150010?

[Nikita Vasilyev]

(In reply to comment #2)
> Is this the same as bug 150010?

They are different from the user's perspective. I don't know if the underling issue is the same.

[Nikita Vasilyev]

(In reply to comment #3)
> (In reply to comment #2)
> > Is this the same as bug 150010?
> 
> They are different from the user's perspective. I don't know if the
> underling issue is the same.

*underlying issue

[Timothy Hatcher]

This looks like a backend issue where we report an error on the main resource URL with a line number from the script.

[Joseph Pecoraro]

*** Bug 142553 has been marked as a duplicate of this bug. ***

[Joseph Pecoraro]

Created attachment 277277 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 277278 [details]
[IMAGE] Before - Format String

[Joseph Pecoraro]

Created attachment 277279 [details]
[IMAGE] After - Format String

[Joseph Pecoraro]

Including images showing the drive-by format fixes also addressed by this.

[Build Bot]

Comment on attachment 277277 [details]
[PATCH] Proposed Fix

Attachment 277277 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1219701

New failing tests:
fast/layers/no-clipping-overflow-hidden-added-after-transform.html

[Build Bot]

Created attachment 277281 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Joseph Pecoraro]

Comment on attachment 277277 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Controllers/IssueManager.js:45
> +        if (sourceCode instanceof WebInspector.Script)
> +            return issue.url === sourceCode.url || (issue.sourceCodeLocation && issue.sourceCodeLocation.sourceCode === sourceCode);

I think I should drop the url comparison here, or fallback to it if the sourceCodeLocation is unavailable. Otherwise, two dynamically added <script>s will both show this issue, even if the sourceCode specifically matches one of them.

[Timothy Hatcher]

Comment on attachment 277277 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Controllers/IssueManager.js:45
>> +            return issue.url === sourceCode.url || (issue.sourceCodeLocation && issue.sourceCodeLocation.sourceCode === sourceCode);
> 
> I think I should drop the url comparison here, or fallback to it if the sourceCodeLocation is unavailable. Otherwise, two dynamically added <script>s will both show this issue, even if the sourceCode specifically matches one of them.

Maybe we should nullify SourceCode's url for dynamic scripts after we associate things? Though it likely makes sense to just drop this compare too.

[Joseph Pecoraro]

<http://trac.webkit.org/changeset/200064>