Bug 195705

Summary:

Web Inspector: Debugger: pausing in an inline script on a page with a URL query creates an Extra Script

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, graouts, hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none

Description Devin Rousso 2019-03-13 15:14:45 PDT

# STEPS TO REPRODUCE:
1. inspect <https://peoplesoakland.mysalononline.com/Booking/?sid=0&guid=fe74f67f-6863-4671-bbb9-704ef70e163e>
2. tap on the date to show the calendar
3. select the <div.k-calendar> node
4. set a breakpoint in the "mousedown" event listener registered on this node
5. Tap anywhere in the calendar

Comment 1 Devin Rousso 2019-03-13 15:14:54 PDT

<rdar://problem/48853820>

Comment 2 Devin Rousso 2019-03-13 15:20:33 PDT

Created attachment 364581 [details]
Patch

Comment 3 Antoine Quint 2019-03-13 15:24:51 PDT

Comment on attachment 364581 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Models/Script.js:280
> +            let mainResource = this._target === WI.mainTarget ? WI.networkManager.mainFrame.mainResource : this._target.mainResource;

Not sure if there's a coding style I don't know about, but technically `const` here would work since mainResource is not re-assigned.

Comment 4 Devin Rousso 2019-03-13 15:26:58 PDT

Comment on attachment 364581 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Models/Script.js:280
>> +            let mainResource = this._target === WI.mainTarget ? WI.networkManager.mainFrame.mainResource : this._target.mainResource;
> 
> Not sure if there's a coding style I don't know about, but technically `const` here would work since mainResource is not re-assigned.

The style that I've known for Web Inspector is that we only use `const` for things that won't change across executions.  We use `const` almost like one would expect `static`.  FWIW, `let` is also less characters than `const` :P

Comment 5 Joseph Pecoraro 2019-03-13 15:38:02 PDT

Comment on attachment 364581 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Models/Script.js:283
> +        if (this._target && !this.isMainResource()) {
> +            let mainResource = this._target === WI.mainTarget ? WI.networkManager.mainFrame.mainResource : this._target.mainResource;
> +            if (mainResource && mainResource.url.startsWith(this._url))
> +                return mainResource;
> +        }

Shouldn't this check each Frame's document resource URLs? What if this was a <script> in a sub-frame's document?

Comment 6 Devin Rousso 2019-03-13 17:56:17 PDT

Created attachment 364598 [details]
Patch

Comment 7 WebKit Commit Bot 2019-03-13 18:35:40 PDT

Comment on attachment 364598 [details]
Patch

Clearing flags on attachment: 364598

Committed r242929: <https://trac.webkit.org/changeset/242929>

Comment 8 WebKit Commit Bot 2019-03-13 18:35:41 PDT

All reviewed patches have been landed.  Closing bug.