Extract Debugger Default prefs

RESOLVED FIXED in Firefox 54

Status

()

Firefox
Developer Tools: Debugger
P2
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jlast, Assigned: jlast)

Tracking

unspecified
Firefox 54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

a year ago
It would be nice if the debugger could manage its own prefs in github.com/devtools-html/debugger.html.

This patch separates out the debugger prefs so that file can be updated when the debugger publishes a new bundle.
(Assignee)

Updated

a year ago
Assignee: nobody → jlaster
(Assignee)

Comment 1

a year ago
Created attachment 8823814 [details] [diff] [review]
dbg-prefs.patch
Attachment #8823814 - Flags: review?(ttromey)

Comment 3

a year ago
Comment on attachment 8823814 [details] [diff] [review]
dbg-prefs.patch

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

Thanks for the patch.  This looks good to me.
I think, however, that the Services shim will need an update to find the new file:
https://dxr.mozilla.org/mozilla-central/rev/57ac9f63fc6953f4efeb0cc84a60192d3721251f/devtools/client/shared/shim/Services.js#438
Also perhaps the inspector webpack config will need a change.
I'm going to clear the review on this basis.
Attachment #8823814 - Flags: review?(ttromey)
(Assignee)

Comment 4

a year ago
Created attachment 8823829 [details] [diff] [review]
dbg-prefs2.patch
Attachment #8823814 - Attachment is obsolete: true
Attachment #8823829 - Flags: review?(ttromey)
(Assignee)

Comment 5

a year ago
Just updated the prefs pr so that Services loads the debugger prefs... but we should find a good way to test this. Perhaps Julian will know how to use the prefs-loader.
Flags: needinfo?(jdescottes)

Comment 6

a year ago
Comment on attachment 8823829 [details] [diff] [review]
dbg-prefs2.patch

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

Thank you, this looks great.
Attachment #8823829 - Flags: review?(ttromey) → review+

Comment 7

a year ago
(In reply to Jason Laster [:jlast] from comment #5)
> Just updated the prefs pr so that Services loads the debugger prefs... but
> we should find a good way to test this. Perhaps Julian will know how to use
> the prefs-loader.

At some point we'll have integration tests for the inspector bundle; without those
we can confidently assume that the de-chroming will regress.  I think though that
we've agreed to defer this until we think everything is more ready.
I agree with Tom, this should be covered by integration tests and I can't see any easy way to test our Services shim against actual prefs.

I hope we can have integration tests for the inspector as soon as possible . The only thing that concerns me with this approach is that in this context the only prefs we would be testing would be inspector prefs.
Flags: needinfo?(jdescottes)
Priority: -- → P2
(Assignee)

Comment 9

a year ago
Created attachment 8830090 [details] [diff] [review]
prefs4.patch
Attachment #8823829 - Attachment is obsolete: true
Attachment #8830090 - Flags: review?(jryans)
(Assignee)

Comment 11

a year ago
Created attachment 8830300 [details] [diff] [review]
prefs5.patch
Attachment #8830090 - Attachment is obsolete: true
Attachment #8830090 - Flags: review?(jryans)
Attachment #8830300 - Flags: review?(jryans)
Comment on attachment 8830300 [details] [diff] [review]
prefs5.patch

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

Looks logical to me, thanks!
Attachment #8830300 - Flags: review?(jryans) → review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 14

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a50557f92553
Extract debugger preferences. r=jryans
Keywords: checkin-needed

Comment 15

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a50557f92553
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
You need to log in before you can comment on or make changes to this bug.