Closed Bug 1435092 Opened 2 years ago Closed 2 years ago

Keep a different set of prefs for the browser console and the web console filter state

Categories

(DevTools :: Console, enhancement, P1)

enhancement

Tracking

(firefox60 fixed)

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: bgrins, Assigned: nchevobbe)

References

Details

(Whiteboard: [newconsole-mvp])

Attachments

(2 files)

We currently keep separate prefs for the two UIs so that if you change a filter in one it doesn't affect the other. This is done by having a copy of each pref, but prefixed with 'browserconsole' instead of 'webconsole': https://dxr.mozilla.org/mozilla-central/rev/7b46ef2ae1412b15ed45e7d2367ca491344729f7/devtools/client/preferences/devtools.js#262.
For the new frontend we currently keep the filter prefs names in constants.js (https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/new-console-output/constants.js#31).

I'm not sure where the right place to inject the variable pref names would be (see callers here: https://dxr.mozilla.org/mozilla-central/search?q=PREFS.FILTER&redirect=true). We could keep another set of constants for the browser console and then have all the callers check hud.isBrowserConsole before accessing the pref value, or have some way to inject prefs into the calling code rather than having them access constants directly. Or, something else I'm not thinking of. Nicolas, any thoughts?
Flags: needinfo?(nchevobbe)
Priority: -- → P2
Ideally, I'd like us to use the same constants in the callers code, no matter if we are in the webconsole or the browserconsole.
Could we imagine something like a pref prefix that would reflect whether or not we are in the browser console ?
All the webconsole pref start with "devtools.webconsole.", so we could have a 

const PREF_PREFIX = isBrowserConsole() ? "devtools.browserconsole" : "devtools.webconsole.";
const prefs = {
  PREFS: {
    FILTER: {
      ERROR: `${PREF_PREFIX}filter.error`,
      WARN: `${PREF_PREFIX}filter.warn`,
      …

What do you think ?
Flags: needinfo?(nchevobbe)
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment on attachment 8950846 [details]
Bug 1435092 - Add a util object to manage preferences; .

https://reviewboard.mozilla.org/r/220080/#review226108

::: devtools/client/webconsole/new-console-output/store.js:82
(Diff revision 2)
>        enableNetProvider(hud)
>      )
>    );
>  }
>  
> +function customThunkMiddleware(options = {}, { dispatch, getState }) {

We are the only consumer of the shared 'thunk' module. We should either delete the module or modify it to suit our needs and continue to use it. Given the size of the module I'd suggest to delete it and then rename the function defined here from `customThunkMiddleware` to just `thunk`
Attachment #8950846 - Flags: review?(bgrinstead) → review+
Comment on attachment 8950968 [details]
Bug 1435092 - Fix mocha tests broken due to changes to prefs; .

https://reviewboard.mozilla.org/r/220228/#review226112

Makes sense to unify with devtools-modules Services shim
Attachment #8950968 - Flags: review?(bgrinstead) → review+
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b0460e52172b
Add a util object to manage preferences; r=bgrins.
https://hg.mozilla.org/integration/autoland/rev/804713551b70
Fix mocha tests broken due to changes to prefs; r=bgrins.
https://hg.mozilla.org/mozilla-central/rev/b0460e52172b
https://hg.mozilla.org/mozilla-central/rev/804713551b70
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.