Closed Bug 1242567 Opened 8 years ago Closed 8 years ago

OptionsPanel.show should use Task to simplify its implementation

Categories

(DevTools :: Framework, defect)

defect
Not set
normal

Tracking

(firefox47 fixed)

RESOLVED FIXED
Firefox 47
Tracking Status
firefox47 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch v1 (obsolete) — Splinter Review
Comment on attachment 8711693 [details] [diff] [review]
patch v1

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

::: devtools/client/framework/toolbox-options.js
@@ +99,3 @@
>      // For local debugging we need to make the target remote.
>      if (!this.target.isRemote) {
> +      this.target.makeRemote();

Should this be yielding?
Attached patch patch v2 (obsolete) — Splinter Review
(In reply to Brian Grinstead [:bgrins] from comment #3)
> Comment on attachment 8711693 [details] [diff] [review]
> patch v1
> 
> Review of attachment 8711693 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/client/framework/toolbox-options.js
> @@ +99,3 @@
> >      // For local debugging we need to make the target remote.
> >      if (!this.target.isRemote) {
> > +      this.target.makeRemote();
> 
> Should this be yielding?

Yes...
Attachment #8711763 - Flags: review?(bgrinstead)
Attachment #8711693 - Attachment is obsolete: true
Comment on attachment 8711763 [details] [diff] [review]
patch v2

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

Works for me, thanks
Attachment #8711763 - Flags: review?(bgrinstead) → review+
Attached patch patch v3Splinter Review
Forgot to also `return this;` at end of open.
Attachment #8712062 - Flags: review+
Attachment #8711763 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/a36366199462
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: