1227137 - about:debugging#addons "enable addons debugging" should also track/toggle devtools.debugger.remote-enabled pref

Status: RESOLVED FIXED

(DevTools :: about:debugging, defect)

Description

[Alexandre Poirot [:ochameau]]

devtools.chrome.enabled pref isn't the only one required to have working addon debugging. We also need devtools.debugger.remote-enabled to be true.

Comment 1

[Julian Descottes [:jdescottes]]

Summary of what was discussed in Bug 1227135 : 
- have one checkbox driving both preferences
- add a "more info" link next to the checkbox label

Some assumptions based on this: 
# Checkbox state :
  - not-checked if one or both preferences are false
  - checked if and only if both preferences are true

# On checkbox change: 
  - when checking the checkbox, both prefs are set to true
  - when unchecking the checkbox, both prefs are set to false

# "more info" link: should open a documentation page? (MDN? Maybe a page already exists?)

:ochameau : let me know if you agree with the statements above, thanks!

Comment 2

[Alexandre Poirot [:ochameau]]

(In reply to Julian Descottes [:jdescottes] from comment #1)
> Summary of what was discussed in Bug 1227135 : 
> - have one checkbox driving both preferences
> - add a "more info" link next to the checkbox label
> 
> Some assumptions based on this: 
> # Checkbox state :
>   - not-checked if one or both preferences are false
>   - checked if and only if both preferences are true
> 
> # On checkbox change: 
>   - when checking the checkbox, both prefs are set to true
>   - when unchecking the checkbox, both prefs are set to false
> 
> # "more info" link: should open a documentation page? (MDN? Maybe a page
> already exists?)

I don't think we have any page on MDN or elsewhere on about:debugging.
But I'm wondering if a tooltip or a shorttext could be enough?

> :ochameau : let me know if you agree with the statements above, thanks!

Sounds good to me.

Also just to be clear. This is a workaround. I prefer seeing a workaround today than keep this weird behavior any longer.

Ideally, we wouldn't even have to toggle any checkbox to have DEBUG button working.
We could also look into why we do need these prefs and if we could make the Addon toolbox work without toggling them. Instead we could somehow spawn a BrowserToolbox with dynamically set flags that would only be used/apply on the toolbox/server instanciated just for the given addon.
It could be worth opening a followup to figure this out.

Comment 3

[Julian Descottes [:jdescottes]]

(In reply to Alexandre Poirot [:ochameau] from comment #2)
> > # "more info" link: should open a documentation page? (MDN? Maybe a page
> > already exists?)
> 
> I don't think we have any page on MDN or elsewhere on about:debugging.
> But I'm wondering if a tooltip or a shorttext could be enough?
> 

IMO, a title is enough (eg. "Turn on/off flags: devtools.debugger.remote-enabled and devtools.chrome.enabled" ?). :janx : I think you were in favor of a "more info" link. Would a title work for you as well?

> > :ochameau : let me know if you agree with the statements above, thanks!
> 
> Sounds good to me.
> 
> Also just to be clear. This is a workaround. I prefer seeing a workaround
> today than keep this weird behavior any longer.
> 
> Ideally, we wouldn't even have to toggle any checkbox to have DEBUG button
> working.
> We could also look into why we do need these prefs and if we could make the
> Addon toolbox work without toggling them. Instead we could somehow spawn a
> BrowserToolbox with dynamically set flags that would only be used/apply on
> the toolbox/server instanciated just for the given addon.
> It could be worth opening a followup to figure this out.

Sounds good. Opened Bug 1248553.

Comment 4

[Jan Keromnes [:janx]]

(In reply to Julian Descottes [:jdescottes] from comment #1)
> # Checkbox state :
>   - not-checked if one or both preferences are false
>   - checked if and only if both preferences are true

Works for me too.

> 
> # On checkbox change: 
>   - when checking the checkbox, both prefs are set to true
>   - when unchecking the checkbox, both prefs are set to false

Note: The unchecking behavior can have confusing side-effects. What if I have exactly one of these prefs enabled (e.g. for some reason I need chrome-debugging, or remote-debugging), and then I enable "Add-on debugging" in about:debugging, and later I re-disable it to "clean up". Now both prefs are disabled, and I'm very confused why my chrome-debugging (or remote-debugging) doesn't work anymore.

In bug 1245029 comment 10, I suggested only disabling one of the prefs on "uncheck", but it's not clear which pref is more important to keep than the other (my suggestion failed to address the following scenario: "I disabled addons in about:debugging, and now I can't open a BrowserToolbox anymore. What happened?" because BrowserToolbox also needs both prefs).

(In reply to Julian Descottes [:jdescottes] from comment #3)
> (In reply to Alexandre Poirot [:ochameau] from comment #2)
> > > # "more info" link: should open a documentation page? (MDN? Maybe a page
> > > already exists?)
> > 
> > I don't think we have any page on MDN or elsewhere on about:debugging.
> > But I'm wondering if a tooltip or a shorttext could be enough?
> > 
> 
> IMO, a title is enough (eg. "Turn on/off flags:
> devtools.debugger.remote-enabled and devtools.chrome.enabled" ?). :janx : I
> think you were in favor of a "more info" link. Would a title work for you as
> well?

First of all, we'll have an MDN page for about:debugging soon (we added a 'dev-doc-needed' keyword to a few related bugs, and I'll add it here). Then, I do prefer a "more info" link to a title/tooltip attribute, simply because the latter is not discoverable: During an early dogfooding session, Sole was very confused by the checkbox, but didn't try hovering over.

I think we need to add a clearly visible link because we're messing with subtle prefs, and this is especially true given my earlier note.

> > Ideally, we wouldn't even have to toggle any checkbox to have DEBUG button
> > working.
> > We could also look into why we do need these prefs and if we could make the
> > Addon toolbox work without toggling them. Instead we could somehow spawn a
> > BrowserToolbox with dynamically set flags that would only be used/apply on
> > the toolbox/server instanciated just for the given addon.
> > It could be worth opening a followup to figure this out.
> 
> Sounds good. Opened Bug 1248553.

Clearly that would be the best solution to our problem. Thanks for filing the bug!

Comment 5

[Julian Descottes [:jdescottes]]

(In reply to Jan Keromnes [:janx] from comment #4)
> (In reply to Julian Descottes [:jdescottes] from comment #1) 
> > 
> > # On checkbox change: 
> >   - when checking the checkbox, both prefs are set to true
> >   - when unchecking the checkbox, both prefs are set to false
> 
> Note: The unchecking behavior can have confusing side-effects. What if I
> have exactly one of these prefs enabled (e.g. for some reason I need
> chrome-debugging, or remote-debugging), and then I enable "Add-on debugging"
> in about:debugging, and later I re-disable it to "clean up". Now both prefs
> are disabled, and I'm very confused why my chrome-debugging (or
> remote-debugging) doesn't work anymore.
> 

I agree this can be an issue, I asked for feedback on the behavior exactly because of this kind of scenarios. The issue is clearly with the uncheck step. From a user point of view, it should act as an "undo". Given this is to be a workaround, I wouldn't invest too much on a complex solution. Still, some threads I explored below:
- have the uncheck to "restore" the state of preferences as they were when about:debugging was opened. But what should it do if all prefs were true on startup, what should it do if prefs where changed from another UI...
- use a button : "Enable addon debugging" with a clear explanation about which preferences it will turn on (more info link? prompt?). Once all prefs are "true" the button would be disabled. The disabled state can be confusing for the user, though.

> (In reply to Julian Descottes [:jdescottes] from comment #3)
> > (In reply to Alexandre Poirot [:ochameau] from comment #2)
> > > > # "more info" link: should open a documentation page? (MDN? Maybe a page
> > > > already exists?)
> > > 
> > > I don't think we have any page on MDN or elsewhere on about:debugging.
> > > But I'm wondering if a tooltip or a shorttext could be enough?
> > > 
> > 
> > IMO, a title is enough (eg. "Turn on/off flags:
> > devtools.debugger.remote-enabled and devtools.chrome.enabled" ?). :janx : I
> > think you were in favor of a "more info" link. Would a title work for you as
> > well?
> 
> First of all, we'll have an MDN page for about:debugging soon (we added a
> 'dev-doc-needed' keyword to a few related bugs, and I'll add it here). Then,
> I do prefer a "more info" link to a title/tooltip attribute, simply because
> the latter is not discoverable: During an early dogfooding session, Sole was
> very confused by the checkbox, but didn't try hovering over.
> 
> I think we need to add a clearly visible link because we're messing with
> subtle prefs, and this is especially true given my earlier note.
> 

Thinking more about it I totally agree. I don't want to overcomplicate what we do here before tackling Bug 1248553. -But- we need to clearly document (and highlight) this tricky behavior. Are we the ones writing the doc pages or is it done by technical writers?

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: bug1227137.wip.patch

This patch implements the feature as described in comment #1 
A "more info" link is displayed but redirects to some MDN page for now.

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: MozReview Request: Bug 1227137 - aboutdebugging: addons debugging enabled only if remote dbg is true;r=ochameau

Review commit: https://reviewboard.mozilla.org/r/35913/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35913/

Comment 8

[Alexandre Poirot [:ochameau]]

Looks good, I just don't know what is the best practice regarding mdn links and localization?
Ryan, Is it ok to refer to an en-US link, or should we get it from a .properties?
Or should we use some more magic url like:
  https://developer.mozilla.org/docs/Tools/about:debugging#Enabling_add-on_debugging

> const MORE_INFO_URL = "https://developer.mozilla.org/en-US/docs/Tools" +
                      "/about:debugging#Enabling_add-on_debugging";
> ...
> React.createElement("a", { href: MORE_INFO_URL, target: "_blank" },
>            Strings.GetStringFromName("addonDebugging.moreInfo"))

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Alexandre Poirot [:ochameau] from comment #8)
> Looks good, I just don't know what is the best practice regarding mdn links
> and localization?
> Ryan, Is it ok to refer to an en-US link, or should we get it from a
> .properties?
> Or should we use some more magic url like:
>  
> https://developer.mozilla.org/docs/Tools/about:debugging#Enabling_add-
> on_debugging
> 
> > const MORE_INFO_URL = "https://developer.mozilla.org/en-US/docs/Tools" +
>                       "/about:debugging#Enabling_add-on_debugging";
> > ...
> > React.createElement("a", { href: MORE_INFO_URL, target: "_blank" },
> >            Strings.GetStringFromName("addonDebugging.moreInfo"))

Use the "magic" version without locale so it will redirect.

See also https://dxr.mozilla.org/mozilla-central/search?q=https%3A%2F%2Fdeveloper.mozilla.org%2Fdocs%2FTools&case=true

Comment 10

[Alexandre Poirot [:ochameau]]

Comment on attachment 8722219 [details]
MozReview Request: Bug 1227137 - aboutdebugging: addons debugging enabled only if remote dbg is true;r=ochameau

https://reviewboard.mozilla.org/r/35913/#review32657

Looks good with the URL to mdn tweaked.

Comment 11

[Julian Descottes [:jdescottes]]

Comment on attachment 8722219 [details]
MozReview Request: Bug 1227137 - aboutdebugging: addons debugging enabled only if remote dbg is true;r=ochameau

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35913/diff/1-2/

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8b7654a6dbb5

Comment 13

[Julian Descottes [:jdescottes]]

Thanks for the review! Updated the URL to the "magic" version.
Try is green, except for 2 hopefully unrelated e10s crashes.
Pushed to fx-team.

Comment 14

[Lina Butler [:lina]]

https://hg.mozilla.org/mozilla-central/rev/8b7654a6dbb5

Comment 15

[Stefan Plewako [:stef]]

> addons = Add-ons
> addonDebugging.label = Enable add-on debugging
> addonDebugging.tooltip = Turning this on will allow you to debug add-ons and various other parts of the browser chrome
>+addonDebugging.moreInfo = more info
> loadTemporaryAddon = Load Temporary Add-on
> extensions = Extensions
> selectAddonFromFile = Select Add-on Directory or XPI File

That https://hg.mozilla.org/mozilla-central/diff/8b7654a6dbb5/devtools/client/locales/en-US/aboutdebugging.properties is not good idea to achieve "Enable add-on debugging (more info)".

Comment 16

[Will Bamberg [:wbamberg]]

This is done: https://developer.mozilla.org/en-US/docs/Tools/about:debugging#Connect_the_Add-on_Debugger

Comment 17

[Saheda Reza [:Antora]]

I have reproduced this on nightly 45.0a1 according to (2015-11-23)

It's fixed on Latest Developer Edition -- Bulid ID (20160423004022) ;User Agent: Mozilla/5.0 (Windows NT 6.3; rv:47.0) Gecko/20100101 Firefox/47.0

Tested OS-- Windows8.1 32bit