If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

move `fetch` from actors/script.js to DevToolsUtils.js

RESOLVED FIXED in Firefox 35

Status

()

Firefox
Developer Tools: Debugger
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jlongster, Assigned: jlongster)

Tracking

unspecified
Firefox 36
Points:
---

Firefox Tracking Flags

(firefox35 fixed, firefox36 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
I need to use the same `fetch` in Fever Dream to fetch the HTML of a page, and this seems like a good utility function anyway so lets move it until our utils so everyone can access it.
(Assignee)

Comment 1

3 years ago
Created attachment 8518267 [details] [diff] [review]
1094965.patch
Attachment #8518267 - Flags: review?(nfitzgerald)
Comment on attachment 8518267 [details] [diff] [review]
1094965.patch

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

::: toolkit/devtools/DevToolsUtils.js
@@ +411,5 @@
> +/**
> + * Performs a request to load the desired URL and returns a promise.
> + *
> + * @param aURL String
> + *        The URL we will request.

While you're here, can you add an @param comment for aOptions?
Attachment #8518267 - Flags: review?(nfitzgerald) → review+
(Assignee)

Comment 3

3 years ago
Created attachment 8518280 [details] [diff] [review]
1094965.patch

sweet, done
Attachment #8518267 - Attachment is obsolete: true
(Assignee)

Comment 4

3 years ago
Created attachment 8518305 [details] [diff] [review]
1094965.patch

Tests failed locally; had to import the `NetUtils` dependency as well (still lazily loads)
Attachment #8518280 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Assignee: nobody → jlong
Comment on attachment 8518305 [details] [diff] [review]
1094965.patch

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

::: toolkit/devtools/DevToolsUtils.js
@@ +407,5 @@
>      return temp[aSymbol || aName];
>    });
>  };
> +
> +exports.defineLazyGetter(this, "NetUtil", () => {

Why do you need to export NetUtil from this module?
(Assignee)

Comment 6

3 years ago
try is green https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f839dd1db2b1
(Assignee)

Comment 7

3 years ago
(In reply to Panos Astithas [:past] (overloaded, please needinfo) from comment #5)
> 
> Why do you need to export NetUtil from this module?

I'm not exporting it, but lazy loading it because it's required inside of `fetch`. That's how it was loaded in the other file. Not sure if it's also attaching it to the module object, and effectively exporting it? This depends on that, but I think we want to lazy load it, is there a better way?
Comment on attachment 8518305 [details] [diff] [review]
1094965.patch

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

::: toolkit/devtools/DevToolsUtils.js
@@ +407,5 @@
>      return temp[aSymbol || aName];
>    });
>  };
> +
> +exports.defineLazyGetter(this, "NetUtil", () => {

defineLazyGetter is on exports, NetUtil is on this.
^ That was supposed to be a reply to Panos
D'oh, you are right. Please carry on.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/4c618071bb26
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
(Assignee)

Comment 12

3 years ago
Eddy, just want to make sure this is on the list of patches to apply to gum
https://hg.mozilla.org/mozilla-central/rev/4c618071bb26
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
https://hg.mozilla.org/releases/mozilla-aurora/rev/43846f2b7e0d
status-firefox35: --- → fixed
status-firefox36: --- → fixed
You need to log in before you can comment on or make changes to this bug.