914930 - load source content via Debugger.Source

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

So there are lots of things we will be able to do when we fully embrace Debugger.Source, but in the meantime, we should stop making HTTP requests to load sources and just use Debugger.Source.prototype.text.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: bug-914930-debugger-source-text.patch

https://tbpl.mozilla.org/?tree=Try&rev=1c49f5d25bc3

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 803404 [details] [diff] [review]
bug-914930-debugger-source-text.patch

Review of attachment 803404 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

::: toolkit/devtools/server/actors/script.js
@@ +3614,5 @@
>     *        The generated source url that introduced this source via source map,
>     *        if any.
> +   * @param optional String text
> +   *        The text content of the source, if immediately available.
> +   * @param optional String text

s/text/contentType/

@@ +3659,5 @@
> +    // source is an HTML-embedded <script> tag. Since we don't have an API
> +    // implemented to detect whether this is the case, we need to be
> +    // conservative and only use Debugger.Source.prototype.text if we get a
> +    // normal .js file.
> +    if (aScript.url && aScript.url.endsWith(".js")) {

This is OK, but a tad too conservative. It misses URLs that contain request parameters for instance. If you parse it using nsIURL you can get to the fileName part easily (see trimUrl in debugger-panes.js).

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://hg.mozilla.org/integration/fx-team/rev/a79989ddb366

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a79989ddb366

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

This change breaks the Browser Debugger. Can't backout due to tree closure and imminent uplift, but I think that's the proper course of action here, unless we can quickly fix the problem.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Eddy is looking into bug 916845, so let's give him a day or so to see if he can quickly come up with a fix.

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Eddy got a fix in (although he is still working on tests as a follow up).