Closed Bug 1314608 Opened 8 years ago Closed 7 years ago

DevTools addon doesn't update preferences

Categories

(DevTools :: General, enhancement, P3)

Product:
DevTools
Component:
General
Type:
enhancement

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: ochameau, Unassigned)

References

Details

Attachments

(1 file)

Bug 1314608 - Install preferences via devtools addon and start devtools on addon install.
58 bytes, text/x-review-board-request
jryans
: review+
Details 
When using the add-on workflow, any changes made to devtools/client/preferences/devtools.js are ignored.

The add-on should try to load this pref file.
Comment on attachment 8806723 [details]
Bug 1314608 - Install preferences via devtools addon and start devtools on addon install.

https://reviewboard.mozilla.org/r/90072/#review90550

::: devtools/bootstrap.js:24
(Diff revision 1)
>    telemetry.actionOccurred(id);
>  }
>  
> +function readURI(uri) {
> +  let stream = NetUtil.newChannel({
> +    uri: NetUtil.newURI(uri, 'UTF-8'),

Nit: Use "

::: devtools/bootstrap.js:29
(Diff revision 1)
> +    uri: NetUtil.newURI(uri, 'UTF-8'),
> +    loadUsingSystemPrincipal: true}
> +  ).open2();
> +  let count = stream.available();
> +  let data = NetUtil.readInputStreamToString(stream, count, {
> +    charset: 'UTF-8'

Nit: Use "

::: devtools/bootstrap.js:48
(Diff revision 1)
> +
> +    // Prevent overriding prefs that have been changed by the user
> +    if (Services.prefs.prefHasUserValue(name)) {
> +      return;
> +    }
> +    if ((val.startsWith("\"") && val.endsWith("\"")) ||

Wouldn't it be more correct to set pref defaults on change here using `Services.prefs.getDefaultBranch`?  That would make it closer to what happens during a regular build with this file.

::: devtools/bootstrap.js:56
(Diff revision 1)
> +    } else if (val.match(/[0-9]+/)) {
> +      Services.prefs.setIntPref(name, parseInt(val));
> +    } else if (val == "true" || val == "false") {
> +      Services.prefs.setBoolPref(name, val == "true");
> +    } else {
> +      dump("unable to match type: "+val+"\n");

Remove or convert to console call.
Attachment #8806723 - Flags: review?(jryans)
Severity: normal → enhancement
Priority: -- → P3
Summary: Devtools addon doesn't update preferences → DevTools addon doesn't update preferences
I switched to use default branch and also process the debugger preferences file.
I think at some point I would move parts of bootstrap.js, like this preference code into JS modules,
as anything living in bootstrap.js is not updated on Ctrl+Alt+R.

Note that Ctrl+Alt+R is still not very well maintained. Mostly because of toolboxes not closing correctly.
For go faster purpose I want the addon to work correctly on install but won't necessarely focus on making the hotreload to work ASAP.
Comment on attachment 8806723 [details]
Bug 1314608 - Install preferences via devtools addon and start devtools on addon install.

https://reviewboard.mozilla.org/r/90072/#review125136

Thanks, this looks like a good improvement!

::: devtools/bootstrap.js:262
(Diff revisions 1 - 2)
>    "devtools.debugger.prompt-connection": false,
>  };
>  let originalPrefValues = {};
>  
>  let listener;
> +let resourceURI;

Maybe it's less mysterious to move this above the function that uses it?
Attachment #8806723 - Flags: review?(jryans) → review+
(In reply to J. Ryan Stinnett [:jryans] (use ni?) (on PTO until Mar. 28) from comment #9)
> Comment on attachment 8806723 [details]
> ::: devtools/bootstrap.js:262
> (Diff revisions 1 - 2)
> >    "devtools.debugger.prompt-connection": false,
> >  };
> >  let originalPrefValues = {};
> >  
> >  let listener;
> > +let resourceURI;
> 
> Maybe it's less mysterious to move this above the function that uses it?

Yes, I also moved listener and originalPrefValues while adding some comment about each one.
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/97f43e645459
Install preferences via devtools addon and start devtools on addon install. r=jryans
https://hg.mozilla.org/mozilla-central/rev/97f43e645459
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: