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

[Devin Rousso]

# 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

[Devin Rousso]

<rdar://problem/48853820>

[Devin Rousso]

Created attachment 364581 [details]
Patch

[Antoine Quint]

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.

[Devin Rousso]

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

[Joseph Pecoraro]

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?

[Devin Rousso]

Created attachment 364598 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 364598 [details]
Patch

Clearing flags on attachment: 364598

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

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.