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

RESOLVED FIXED in Firefox 60

Status

enhancement
P1
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: bgrins, Assigned: nchevobbe)

Tracking

Trunk
Firefox 60
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

(Whiteboard: [newconsole-mvp])

Attachments

(2 attachments)

Reporter

Description

a year ago
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.
Reporter

Comment 1

a year ago
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
Assignee

Comment 2

a year ago
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

Updated

a year ago
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Reporter

Comment 7

a year ago
mozreview-review
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+
Reporter

Comment 8

a year ago
mozreview-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+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 18

a year ago
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.

Comment 19

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b0460e52172b
https://hg.mozilla.org/mozilla-central/rev/804713551b70
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60

Updated

a year ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.