Closed Bug 1227137 Opened 9 years ago Closed 9 years ago

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

Categories

(DevTools :: about:debugging, defect)

defect
Not set
normal

Tracking

(firefox47 fixed)

RESOLVED FIXED
Firefox 47
Tracking Status
firefox47 --- fixed

People

(Reporter: ochameau, Assigned: jdescottes)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

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.
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Depends on: 1245029
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!
Flags: needinfo?(poirot.alex)
(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.
Flags: needinfo?(poirot.alex)
(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.
Flags: needinfo?(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!
Flags: needinfo?(janx)
Keywords: dev-doc-needed
(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?
Attached patch bug1227137.wip.patch (deleted) — Splinter Review
This patch implements the feature as described in comment #1 A "more info" link is displayed but redirects to some MDN page for now.
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"))
Flags: needinfo?(jryans)
(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
Flags: needinfo?(jryans)
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.
Attachment #8722219 - Flags: review?(poirot.alex) → review+
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/
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.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
> 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)".
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
QA Whiteboard: [testday-20160422]
QA Whiteboard: [testday-20160422] → [bugday-20160420]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: