The current status of the Addon Debugging is still far from perfect, after discussing with Will Bamberg we agreed that, besides working on fixing bugs and make addon debugging awesome in the next releases, an helpful first step could be adding a link to an MDN doc page specifically created to guide the add-on developer and suggest to him the current (and the next) best practices. The attached screenshot shows how the current proposal looks like (I took the screenshot from a Nightly built with the attached patch applied): - the new link is visible only when the add-ons debugging is enabled - the text and the MDN url are both taken from the localized strings (which can be useful to provide a different localized url and description on other locales)

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

Comment on attachment 8764230 [details] Bug 1281451 - Propose an Add-ons Debugging doc page on MDN from about:debugging. Set f? for an initial feedback on both the proposal itself, the current proposed change and the related test assertions (it is my first time on the about:debugging component).

Didn't we also discuss making the "Debug" buttons open the Browser Toolbox, not the Add-on Debugger (at least for WebExtensions, and possibly for other types as well)? Part of the problem here is that the WebExtension docs recommend that people use the Browser Toolbox, but these buttons launch the Add-on Debugger, which must be confusing for people.

(In reply to Will Bamberg [:wbamberg] from comment #3) > Didn't we also discuss making the "Debug" buttons open the Browser Toolbox, > not the Add-on Debugger (at least for WebExtensions, and possibly for other > types as well)? > > Part of the problem here is that the WebExtension docs recommend that people > use the Browser Toolbox, but these buttons launch the Add-on Debugger, which > must be confusing for people. Yep, That was part of what we discussed, but it is a more controversial change and so I thought to open it as a separate proposal from this one (which is smaller and less controversial change) and unfortunately we have to consider that there are scenarios where the Browser Toolbox currently fails to provide a reasonable solution, e.g. a breakpoint (even introduced using the `debugger` statement) in a WebExtension script injected in a popup fails to fire and pause the debugger (In the last days I've tried to reproduce it because I'm going to open a separate bugzilla issue for it, and it happens consistently). My concern is that by opening the BrowserToolbox by default when the "debug" button is pressed on a WebExtension, we are going to prevent to any add-on developer to reach the workaround (even if I totally agree that "using two different developer toolboxes" is definitely a very annoying workaround).

Comment on attachment 8764227 [details] aboutdebugging_addon_mdn_link_proposal.png Thank you for this proposal Luca! Having a "learn more" link is a great idea. However, I'm not sure what the best location for it is. I think personally I would prefer it bottom right of the page, for example. Helen, what do you think about placing the "Learn more..." link, either just below the "Enable add-on debugging" checkbox as in the screenshot, or down at the bottom right of the "Add-ons" panel?

https://reviewboard.mozilla.org/r/60234/#review57200 Thank you for this patch Luca! I'm happy to review it (MozReview still doesn't know f+, so I'll set it manually) but in general :ochameau or :kumar are the right people to ask for reviews to the Add-ons panel. (In reply to Will Bamberg [:wbamberg] from comment #3) > Didn't we also discuss making the "Debug" buttons open the Browser Toolbox, > not the Add-on Debugger (at least for WebExtensions, and possibly for other > types as well)? > > Part of the problem here is that the WebExtension docs recommend that people > use the Browser Toolbox, but these buttons launch the Add-on Debugger, which > must be confusing for people. I'm slightly confused here, because about:debugging calls `BrowserToolboxProcess.init({ addonID })` to open toolboxes on add-ons[0]. Is what we call an "Add-on Debugger" not a special kind of Browser Toolbox? In any case, :ochameau and :kumar will know more about these things. [0] https://dxr.mozilla.org/mozilla-central/source/devtools/client/aboutdebugging/components/addons/target.js#25 ::: devtools/client/aboutdebugging/components/addons/controls.js:91 (Diff revision 1) > dom.button({ > id: "load-addon-from-file", > onClick: this.loadAddonFromFile, > }, Strings.GetStringFromName("loadTemporaryAddon")) > ), > + debugDisabled ? [] : dom.div( Oh, I see that you're trying to display the link only when Add-on debugging is enabled. Is there any particular reason for not always showing it? I would prefer if documentation links are always visible and part of the UI, regardless of any state. ::: devtools/client/aboutdebugging/test/browser_addons_toggle_debug.js:34 (Diff revision 1) > let addonDebugCheckbox = document.querySelector("#enable-addon-debugging"); > ok(!addonDebugCheckbox.checked, "Addons debugging should be disabled."); > > + info("Check the Addon Debugging help section is not visible."); > + let addonDebuggingHelp = document.querySelector(".addons-debugging-info"); > + ok(!addonDebuggingHelp, "Addon Debugging help section should not exist."); No need to enforce this.

(In reply to Jan Keromnes [:janx] from comment #5) > Comment on attachment 8764227 [details] > aboutdebugging_addon_mdn_link_proposal.png > > Thank you for this proposal Luca! > > Having a "learn more" link is a great idea. However, I'm not sure what the > best location for it is. I think personally I would prefer it bottom right > of the page, for example. > > Helen, what do you think about placing the "Learn more..." link, either just > below the "Enable add-on debugging" checkbox as in the screenshot, or down > at the bottom right of the "Add-ons" panel? Hm, I think it makes sense to place it underneath add-ons directly (as it is in the screenshot) if it refers to add-ons specifically. Placing it at the bottom of the panel would send the message that the advice was more general, I believe. We've chatted some on the UX team about how people find add-ons/extensions/themes/web-extensions pretty confusing even internally, so I think it makes sense to be as clear as we feel we can here.

Comment on attachment 8764227 [details] aboutdebugging_addon_mdn_link_proposal.png (Feedback in comment #7.)

Thanks Jan, it was exactly the kind of initial feedback that I needed. (In reply to Jan Keromnes [:janx] from comment #6) > I'm slightly confused here, because about:debugging calls > `BrowserToolboxProcess.init({ addonID })` to open toolboxes on add-ons[0]. > Is what we call an "Add-on Debugger" not a special kind of Browser Toolbox? > In any case, :ochameau and :kumar will know more about these things. Exactly, when `BrowserToolboxProcess.init` is called with an addonID, the toolbox is trimmed down to what it is currently supported in the `addon` actor. > ::: devtools/client/aboutdebugging/components/addons/controls.js:91 > (Diff revision 1) > > dom.button({ > > id: "load-addon-from-file", > > onClick: this.loadAddonFromFile, > > }, Strings.GetStringFromName("loadTemporaryAddon")) > > ), > > + debugDisabled ? [] : dom.div( > > Oh, I see that you're trying to display the link only when Add-on debugging > is enabled. Is there any particular reason for not always showing it? I would be happy with always showing it (and I think Will will be happy too), I'm going to tweak this patch accordingly. > I would prefer if documentation links are always visible and part of the UI, > regardless of any state. > > ::: devtools/client/aboutdebugging/test/browser_addons_toggle_debug.js:34 > (Diff revision 1) > > let addonDebugCheckbox = document.querySelector("#enable-addon-debugging"); > > ok(!addonDebugCheckbox.checked, "Addons debugging should be disabled."); > > > > + info("Check the Addon Debugging help section is not visible."); > > + let addonDebuggingHelp = document.querySelector(".addons-debugging-info"); > > + ok(!addonDebuggingHelp, "Addon Debugging help section should not exist."); > > No need to enforce this. sounds good to me.

(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #7) > (In reply to Jan Keromnes [:janx] from comment #5) > > Comment on attachment 8764227 [details] > > aboutdebugging_addon_mdn_link_proposal.png > > > > Thank you for this proposal Luca! > > > > Having a "learn more" link is a great idea. However, I'm not sure what the > > best location for it is. I think personally I would prefer it bottom right > > of the page, for example. > > > > Helen, what do you think about placing the "Learn more..." link, either just > > below the "Enable add-on debugging" checkbox as in the screenshot, or down > > at the bottom right of the "Add-ons" panel? > > Hm, I think it makes sense to place it underneath add-ons directly (as it is > in the screenshot) if it refers to add-ons specifically. Placing it at the > bottom of the panel would send the message that the advice was more general, > I believe. sounds great! Thanks Helen for the fast feedback. I tried to follow my rule: "no css rule were harmed in the making of" the initial proposal :-) and "staying in the safe" by keeping it simple and re-using something that was already in the existent css file (and then wait for the feedback by who is way better and more qualified than me in taking this kind of decisions)

Comment on attachment 8764230 [details] Bug 1281451 - Propose an Add-ons Debugging doc page on MDN from about:debugging. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60234/diff/1-2/

(In reply to Luca Greco [:rpl] from comment #11) > Comment on attachment 8764230 [details] > Bug 1281451 - Propose an Add-ons Debugging doc page on MDN from > about:debugging. > > Review request updated; see interdiff: > https://reviewboard.mozilla.org/r/60234/diff/1-2/ Pushed updated patch with the following tweaks: - the link is always visible (and the change and related test fixed/simplified accordingly) - the link is positioned and styled as in the attached screenshot

(In reply to Jan Keromnes [:janx] from comment #6) > https://reviewboard.mozilla.org/r/60234/#review57200 > > (In reply to Will Bamberg [:wbamberg] from comment #3) > > Didn't we also discuss making the "Debug" buttons open the Browser Toolbox, > > not the Add-on Debugger (at least for WebExtensions, and possibly for other > > types as well)? > > > > Part of the problem here is that the WebExtension docs recommend that people > > use the Browser Toolbox, but these buttons launch the Add-on Debugger, which > > must be confusing for people. > > I'm slightly confused here, because about:debugging calls > `BrowserToolboxProcess.init({ addonID })` to open toolboxes on add-ons[0]. > Is what we call an "Add-on Debugger" not a special kind of Browser Toolbox? > In any case, :ochameau and :kumar will know more about these things. > I don't know anything about the internals, but from the outside perspective there are 2 main reasons that the docs recommend people to use the Browser Toolbox rather than the Add-on Debugger: * you can't use the Add-on Debugger to debug layout problems in popups, because it doesn't have an inspector. By contrast, the workflow for the BT is reasonably nice now: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Debugging#Debugging_popups * the Add-on Debugger's console does not show error messages from the add-on's code, the Browser Toolbox does. If I'm wrong about either of these, I'd be very glad to hear it. If not, I think recommending the BT in the docs is the right thing to do. But then if the docs do recommend the BT, there's a weird dissonance with about:debugging launching the Add-on Debugger. So my suggestion was that about:debugging should launch the BT instead, *and* include a link to the docs which explain how to use the BT. However, if about:debugging still launches the Add-on Debugger (per comment 4), then I'm less sure about the value of this bug. If we include a link to the docs, and then the docs tell people to use the BT instead, won't that be really weird?

(In reply to Will Bamberg [:wbamberg] from comment #13) > I don't know anything about the internals, but from the outside perspective > there are 2 main reasons that the docs recommend people to use the Browser > Toolbox rather than the Add-on Debugger: > > * you can't use the Add-on Debugger to debug layout problems in popups, > because it doesn't have an inspector. By contrast, the workflow for the BT > is reasonably nice now: > https://developer.mozilla.org/en-US/Add-ons/WebExtensions/ > Debugging#Debugging_popups > > * the Add-on Debugger's console does not show error messages from the > add-on's code, the Browser Toolbox does. > > If I'm wrong about either of these, I'd be very glad to hear it. If not, I > think recommending the BT in the docs is the right thing to do. But then if > the docs do recommend the BT, there's a weird dissonance with > about:debugging launching the Add-on Debugger. Confirmed, you are completely right on this. > So my suggestion was that about:debugging should launch the BT instead, > *and* include a link to the docs which explain how to use the BT. > > However, if about:debugging still launches the Add-on Debugger (per comment > 4), then I'm less sure about the value of this bug. If we include a link to > the docs, and then the docs tell people to use the BT instead, won't that be > really weird? Hi Will, I'm very sorry, in Comment 4 I probably failed to explain it clearly enough: I wasn't suggesting that opening the Browser Toolbox instead of the Add-on Debugger is totally excluded from my point of view (and in fact immediatelly after looking into this, I took a look at how to do it so that we can create a proposal for it). My point is that if we decide to open the Browser Toolbox by default when the Debug button has been clicked on a WebExtension add-on, then we have to provide a way to optionally open the Add-on Debugger (because, at least currently, it is the only way to be able to pause on a breakpoint some of the WebExtensions contexts and be able to debug it, I agree that the DOM Inspector panel in most of the cases is probably used a lot more than the Debugger panel, but we have to be sure that it is still possible). I though to create the issue that adds the link to an MDN page as a first step, because I think that it can be helpful in any case, because we have the opportunity to control the landing page that the add-on developer is going to reach to get more information (and right now we probably have to hope that they will end up on the right MDN page, and not on the wrong one or on a stackoverflow question/answer). And even better, the linked MDN page can be updated across the releases and describe the correct workflow for every release that we think it is important to be covered explicitly (and how to workaround any bug that exists in that version).

> I though to create the issue that adds the link to an MDN page as a first > step, because I think that it can be helpful in any case, because we have > the opportunity to control the landing page that the add-on developer is > going to reach to get more information (and right now we probably have to > hope that they will end up on the right MDN page, and not on the wrong one > or on a stackoverflow question/answer). > > And even better, the linked MDN page can be updated across the releases and > describe the correct workflow for every release that we think it is > important to be covered explicitly (and how to workaround any bug that > exists in that version). Having discussed this with you, Luca: I think you're right that linking to some docs here would be useful. My worry is that I'm not sure what the docs should actually say, since the story is quite convoluted at the moment. I'd like our story to be more compelling, but I appreciate that we are not there yet, and it would probably be better to tell a convoluted story than none at all. So I'll think a bit, and outline in this bug what docs we could have at the end of this link.

Comment on attachment 8764230 [details] Bug 1281451 - Propose an Add-ons Debugging doc page on MDN from about:debugging. Oh, I guess you're expecting me to review this again, but MozReview failed to actually set the flag. I'll review your patch shortly. (In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #7) > Hm, I think it makes sense to place it underneath add-ons directly (as it is > in the screenshot) if it refers to add-ons specifically. Placing it at the > bottom of the panel would send the message that the advice was more general, > I believe. Well, I believe the article will be "generally about add-ons", hence my preference for placing it bottom-right of the Add-ons panel. But maybe you're implying that a bottom-right link looks more like "generally about about:debugging (the whole tool)" than "generally about add-ons (this panel)"? > We've chatted some on the UX team about how people find > add-ons/extensions/themes/web-extensions pretty confusing even internally, > so I think it makes sense to be as clear as we feel we can here. Agreed, these are really confusing. I have two ideas to make this a bit clearer: 1) Currently about:debugging only shows one "kind" of Add-on in this panel, and the single sub-category is called "Extensions". Maybe it would be clearer to separate add-ons into more specific sub-categories, like "Web Extensions", "Browser Add-ons", "Themes", etc? (Similar to the Worker panel's "Service Workers", "Shared Workers" and "Other" sub-categories). Then we could also have links to MDN articles dedicated to each specific add-on "kind": e.g. "Web Extensions (more info)", with "more info" being a link to an MDN section about "Web Extensions", etc. I feel such links would be more compelling than a single "learn more about all this add-on debugging thing", which to me implies a big and convoluted article. But let's go ahead with the single link for now, as it's already a big improvement. 2) Maybe our MDN article could include tables with "✓" and "✕", to show exactly what each type of toolbox can do with each type of add-on (to make our convoluted story a bit simpler). From what I understand: Browser Toolbox can: - show errors from add-on code in console - inspect add-on popups layout in inspector Add-on Debugger can: - pause on a breakpoint in add-on code, and step forward Anything else? Does "add-on" here mean "Web Extension", "Bootstrapped Add-on", "Theme", or a combination of them? (In reply to Luca Greco [:rpl] from comment #14) > I wasn't suggesting that opening the Browser Toolbox instead of the Add-on > Debugger is totally excluded from my point of view (and in fact immediatelly > after looking into this, I took a look at how to do it so that we can create > a proposal for it). Exciting! It would be very cool to make about:debugging's Debug buttons open a BrowserToolbox that has more features. You can ask :ochameau for help if you're not sure how to launch a BrowserToolbox targeted to the correct add-on frame/context. > My point is that if we decide to open the Browser Toolbox by default when > the Debug button has been clicked on a WebExtension add-on, then we have to > provide a way to optionally open the Add-on Debugger If we really go that way, maybe we can add a tooltip to the Debug button that says "Debug with Browser Toolbox", and add a dropdown button next to it (e.g. "▼") that offers more options like "Debug with Add-on Debugger", and maybe "Reload", "Disable", "Uninstall"...

Comment on attachment 8764230 [details] Bug 1281451 - Propose an Add-ons Debugging doc page on MDN from about:debugging. https://reviewboard.mozilla.org/r/60234/#review57856 Looks good to me in general (with one small nit), thanks for the patch! But I think Kumar would be a better reviewer for this change, since he did a lot of work around this code. ::: devtools/client/aboutdebugging/components/addons/controls.js:92 (Diff revision 2) > id: "load-addon-from-file", > onClick: this.loadAddonFromFile, > }, Strings.GetStringFromName("loadTemporaryAddon")) > ), > + dom.div( > + { className: "addons-debugging-info"}, Nit: Local style is to put objects on a single line when possible (e.g. `dom.div({ className },`) or to split every attribute over new lines if too big, e.g. dom.div({ className, param2, param3, param4, }

Comment on attachment 8764230 [details] Bug 1281451 - Propose an Add-ons Debugging doc page on MDN from about:debugging. Please have a look.

(In reply to Jan Keromnes [:janx] from comment #16) > Comment on attachment 8764230 [details] > (In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #7) > > Hm, I think it makes sense to place it underneath add-ons directly (as it is > > in the screenshot) if it refers to add-ons specifically. Placing it at the > > bottom of the panel would send the message that the advice was more general, > > I believe. > > Well, I believe the article will be "generally about add-ons", hence my > preference for placing it bottom-right of the Add-ons panel. But maybe > you're implying that a bottom-right link looks more like "generally about > about:debugging (the whole tool)" than "generally about add-ons (this > panel)"? If the link refers to debugging add-ons, extensions, and everything that is on the page, then placing it in the bottom-right makes sense. If the link /only/ refers to debugging add-ons, I think it makes sense to have it underneath add-ons. > > We've chatted some on the UX team about how people find > > add-ons/extensions/themes/web-extensions pretty confusing even internally, > > so I think it makes sense to be as clear as we feel we can here. > > Agreed, these are really confusing. I have two ideas to make this a bit > clearer: > > 1) Currently about:debugging only shows one "kind" of Add-on in this panel, > and the single sub-category is called "Extensions". Maybe it would be > clearer to separate add-ons into more specific sub-categories, like "Web > Extensions", "Browser Add-ons", "Themes", etc? (Similar to the Worker > panel's "Service Workers", "Shared Workers" and "Other" sub-categories). > > Then we could also have links to MDN articles dedicated to each specific > add-on "kind": e.g. "Web Extensions (more info)", with "more info" being a > link to an MDN section about "Web Extensions", etc. > > I feel such links would be more compelling than a single "learn more about > all this add-on debugging thing", which to me implies a big and convoluted > article. This might be worth asking Michelle Heubusch about eventually (she's our content strategist). In the meantime, however, I agree that having a link is better than no link.

Comment on attachment 8764230 [details] Bug 1281451 - Propose an Add-ons Debugging doc page on MDN from about:debugging. https://reviewboard.mozilla.org/r/60234/#review57890 Looks good to me if the URL is OK (see review comment). ::: devtools/client/locales/en-US/aboutdebugging.properties:16 (Diff revision 2) > > 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 > +addonDebuggingHelp.text = Learn more about how to debug add-ons > +addonDebuggingHelp.link = https://developer.mozilla.org/en-US/Add-ons/Debugging This is a 404 for me. Has it just not been created yet?

Comment on attachment 8764230 [details] Bug 1281451 - Propose an Add-ons Debugging doc page on MDN from about:debugging. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60234/diff/2-3/

https://reviewboard.mozilla.org/r/60234/#review57856 > Nit: Local style is to put objects on a single line when possible (e.g. `dom.div({ className },`) or to split every attribute over new lines if too big, e.g. > > dom.div({ > className, > param2, > param3, > param4, > } That's true, I tweaked this line to use the same style of the other elements.

https://reviewboard.mozilla.org/r/60234/#review57890 > This is a 404 for me. Has it just not been created yet? Will is creating an outline of the docs that we are going to have at that link (as from Comment 15), but before proceed to clear this issue from mozreview and set the checkin-needed, I'll wait a final confirmation that the url is right from Will.

Hi Will, is the following url right? https://developer.mozilla.org/en-US/Add-ons/Debugging I'll wait your confirmation, then I'm going to set the checkin-needed to proceed to land this patch. Any issues on landing this patch even if the page is not immediatelly there? (but ensuring that we will have an actual page before that current nightly, 50, moves to "Aurora")

Sorry I've been so slow at answering this. Here's more or less what I think. I could write a page on MDN that tries to explain the differences between the different toolboxes, and in particular, that even though the "Debug" page launches the Add-on Debugger, developer should probably be using the Browser Toolbox instead. I think given the current situation this would add value for developers. But I'm not likely to have the time to write this for the next few weeks, as I'm still quite busy trying to get the WebExtension docs in reasonable shape for Firefox 48 (and 49). I'd also much rather we make the debugging experience better, than document the shortcomings of the current experience. I think the work Luca's been doing is looking really promising, and to me it makes more sense to try to land that, then document it as the one true way to debug restartless add-ons.

It looks like https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Debugging has been added. Would this be a simple matter now of adding a link to this page?

I think this exists and we can close this bug.

Closing as fixed (as it can be seen in screenshot from Comment 27, there is a learn more link in the about:debugging page now).