Closed Bug 1425347 Opened 3 years ago Closed 2 years ago

Separate user add-ons and system add-ons

Categories

(DevTools :: about:debugging, enhancement, P4)

enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: mstriemer, Assigned: mstriemer)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

System add-ons and user add-ons should be separated in about:debugging. If system add-ons are hidden by default that would be better.


Temporary add-ons

  My add-on

Add-ons

  Another add-on

System add-ons

  [Show]
Similar to what was discussed in Bug 1284711.

I'm fine with keeping the two bugs as separate, because this one is more focused, but there are some interesting discussions in the other bug.
See Also: → 1284711
Comment on attachment 8981270 [details]
Bug 1425347 - Hide system add-ons by default in about:debugging

https://reviewboard.mozilla.org/r/247364/#review253696

Thanks for the patch Mark, the implementation looks good, but I'm not sure if we should add a checkbox to enable it.
"Show system extensions" will probably not make sense to users that have never heard of system addons (and googling it doesn't really help).

So either:
- have a learn more link (not sure where it should point to), maybe find a more generic label? maybe have a one liner below the checkbox explaining what thoses extensions are
- simply no checkbox, users who want to debug system addons have to enable it in about:config (we could make the pref true by default on local builds)

Overall I feel like having a UI would benefit a small fraction of users, and we can probably reach out to them directly to let them know that system addons are behind a pref in about:debugging now.
Let me know what you think or if there were prior discussions about this.

::: devtools/client/aboutdebugging/test/browser_addons_debug_info.js:123
(Diff revision 1)
> +  await SpecialPowers.pushPrefEnv({
> +    set: [["devtools.debugger.showSystemAddons", false]],
> +  });

you should be able to use the alias pushPref(prefName, prefValue) coming from DevTools' shared-head.js

::: devtools/shared/preferences/devtools-shared.js:33
(Diff revision 1)
>  pref("devtools.chrome.enabled", true, sticky);
>  pref("devtools.debugger.remote-enabled", true, sticky);
>  #endif
>  
> +// Hide system add-ons in about:debugging
> +pref("devtools.debugger.showSystemAddons", false, sticky);

Since this preference is only relevant to about:debugging (which is part of devtools/client) it should be defined in devtools/client/preferences/devtools-client.js.

Its name should be devtools.aboutdebugging.showSystemAddons (rather than "debugger")

Finally I don't know if the sticky parameter was intentional or copy paste?
Attachment #8981270 - Flags: review?(jdescottes)
I removed the checkbox so you need to change the pref to see system add-ons if you're not on a local build now.

Including a string at the bottom that links to a SUMO page seems good to me but I don't know what the wording would be or what content to put on that page. I'll try and figure that out next week if that works. Happy to do something else as well.
Product: Firefox → DevTools
Comment on attachment 8981270 [details]
Bug 1425347 - Hide system add-ons by default in about:debugging

https://reviewboard.mozilla.org/r/247364/#review257202

Looks good to me, thanks! 

Maybe we should just announce this somewhere when landing, eg email to dev-platform? 
Not sure what is the best way to reach out to developers working on system addons.

::: devtools/client/aboutdebugging/components/addons/Panel.js:178
(Diff revision 2)
>      },
>      PanelHeader({
>        id: id + "-header",
>        name: Strings.GetStringFromName("addons")
>      }),
> -    AddonsControls({ debugDisabled }),
> +    AddonsControls({ debugDisabled, showSystemAddons }),

we no longer need to pass this

::: devtools/client/locales/en-US/aboutdebugging.properties:56
(Diff revision 2)
> +# LOCALIZATION NOTE (showSystemAddons.tooltip):
> +# This string is displayed in a tooltip that appears when hovering over a check
> +# box that switches showing system add-ons on/off.
> +showSystemAddons.tooltip = Turning this on will show the list of installed system add-ons
> +

We can remove this now
Attachment #8981270 - Flags: review?(jdescottes) → review+
Anything blocking this from landing Mark?
Flags: needinfo?(mstriemer)
It just fell off my radar after some PTO, landing it now, thanks.
Flags: needinfo?(mstriemer)
Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74538c63a6a9
Hide system add-ons by default in about:debugging r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/74538c63a6a9
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
I'm not entirely sure how you want to document this. Is it enough to edit the existing description of the about:debugging page to say that the list doesn't not include system add-ons? Or is there a way to turn on the display of system add-ons that I should be describing? I noticed that the checkbox to turn on the display was removed, that's the reason I'm asking.
Flags: needinfo?(mstriemer)
System add-ons will be hidden based on the devtools.aboutdebugging.showSystemAddons pref which is false in official builds and true in unofficial builds. Flipping that pref will show them if needed.
Flags: needinfo?(mstriemer)
Added this to the release notes:

The default value of devtools.aboutdebugging.showSystemAddons is now false, meaning that system add-ons will not be listed on the about:debugging page. You can change the settings by navigating to about:config. ({{bug(1425347)}}).

and on https://developer.mozilla.org/en-US/docs/Tools/about:debugging$edit, I added this paragraph:

Whether or not system add-ons appear in the list on this page depends on the setting of the devtools.aboutdebugging.showSystemAddons preference. If you need to see system add-ons, navigate to about:config and make sure that this value is set to true.
Flags: needinfo?(mstriemer)
Looks great, thanks!
Flags: needinfo?(mstriemer)
Duplicate of this bug: 1284711
You need to log in before you can comment on or make changes to this bug.