1209344 - remove "debug" buttons on about:addons and refer to about:debugging

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

Developers want to be able to load an add-on temporarily for testing so we need a UI to let them provide a directory to load an add-on from and run.

Comment 1

[Andy McKay]

We were thinking that there would be another option in the add-ons menu gear drop down (see below) that said, something like "Temporarily install unsigned Add-on from File...". Although it might work for a directory or .xpi, but I don't know if that needs differentiating.

https://www.dropbox.com/s/i2hxx91buzsliov/Screenshot%202015-10-26%2010.16.03.png?dl=0

I'm not sure about that message though Markus, any advice?

Comment 2

[Bram Pitoyo [:bram]]

In my opinion, we can do either one of these four things. They all rely on the fact that our devs are able to toggle options that may be obscure to every day user, or use different Firefox channels.

1. Add another link to the drop down menu: “Temporarily install unsigned Add-on from File…”. This is the solution proposed above.
  * This seems to be the most obvious and accessible place to put this link
  * But it may be too accessible to every day user, leading to misuse?

2. Modify the existing “Install Add-on from File…” functionality, so it can install both signed and unsigned add-ons
  * However, this ability only exists in Dev Edition or Beta

3. On the file upload dialogue, have a checkbox that says “Only install signed add-ons” that is checked by default. If unchecked, then unsigned add-ons can be installed

4. Put the option to install unsigned add-ons behind a pref. If this pref is on, then user can select any unsigned add-on using this dialogue, and it will be installed.
  * This may be the most obscure way of triggering this feature, but our devs should be familiar with about:preferences

Personally, I’m in favour of approach #2 or #4. What do you think?

(Clearing Markus’ needinfo, but feel free to put it back on if the answer doesn’t satisfy).

Comment 3

[Markus Jaritz [:designakt] (UX)]

(In reply to Bram Pitoyo [:bram] from comment #2)
> 1. Add another link to the drop down menu: “Temporarily install unsigned
> Add-on from File…”. This is the solution proposed above.
>   * This seems to be the most obvious and accessible place to put this link
>   * But it may be too accessible to every day user, leading to misuse?

I like that way. It seams fairly well hidden so that normal users might not use it. (Close to none of the users we tested so far has ever been to the add-ons manager in their respective browser. They install and use add-ons, but no more.)

Do we need a permanent indicator while a temporary add-on is active? 
(Is it possible to automatically install a temporary add-on?)

(In reply to Andy McKay [:andym] from comment #1)
> We were thinking that there would be another option in the add-ons menu gear
> drop down (see below) that said, something like "Temporarily install
> unsigned Add-on from File...". Although it might work for a directory or
> .xpi, but I don't know if that needs differentiating.

Regarding the message I am not sure if we need the "unsigned" in it, 
but somehow could give a hint that this feature is aimed at devs.
Would be great to get feedback on the wording from :Matej

Options:
"Temporarily install unsigned Add-on from File..."
"Test Add-on temporarily (Install from File)..."
"Install Add-on temporarily from File..."
"Install Add-on temporarily to debug..."
...


> 2. Modify the existing “Install Add-on from File…” functionality, so it can
> install both signed and unsigned add-ons
>   * However, this ability only exists in Dev Edition or Beta

Not possible, as the idea behind that feature is allowing devs to test in release.
 
> 3. On the file upload dialogue, have a checkbox that says “Only install
> signed add-ons” that is checked by default. If unchecked, then unsigned
> add-ons can be installed

In terms of discover-ability this seams very close to option 1. But would change a systems dialog, which probably makes it more complicated to implement.

> 4. Put the option to install unsigned add-ons behind a pref. If this pref is
> on, then user can select any unsigned add-on using this dialogue, and it
> will be installed.
>   * This may be the most obscure way of triggering this feature, but our
> devs should be familiar with about:preferences

I guess if we plan to have more dev-features implemented in release we might consider adding a button to turn on such a developer mode. For one additional line in one sub-menu on add-on manager, I am not sure if need need that.
 
> Personally, I’m in favour of approach #2 or #4. What do you think?

I would go with 1, and if we have multiple dev-features in release, add a pref, or button in settings to enable dev-mode.

> (Clearing Markus’ needinfo, but feel free to put it back on if the answer
> doesn’t satisfy).

Comment 4

[Matej Novak [:matej]]

(In reply to Markus Jaritz [:maritz] (UX) from comment #3)
> (In reply to Bram Pitoyo [:bram] from comment #2)
> (In reply to Andy McKay [:andym] from comment #1)
> > We were thinking that there would be another option in the add-ons menu gear
> > drop down (see below) that said, something like "Temporarily install
> > unsigned Add-on from File...". Although it might work for a directory or
> > .xpi, but I don't know if that needs differentiating.
> 
> Regarding the message I am not sure if we need the "unsigned" in it, 
> but somehow could give a hint that this feature is aimed at devs.
> Would be great to get feedback on the wording from :Matej
> 
> Options:
> "Temporarily install unsigned Add-on from File..."
> "Test Add-on temporarily (Install from File)..."
> "Install Add-on temporarily from File..."
> "Install Add-on temporarily to debug..."
> ...

I agree that you probably don't need "unsigned" here. I like the second and third options above, though the third is probably the clearest:

"Install Add-on temporarily from File..."

Also, is it standard to have "Add-on" capitalized in cases like this? In marketing copy we only capitalize it when it directly follows Firefox, which is why I'm asking.

Comment 5

[Markus Jaritz [:designakt] (UX)]

(In reply to Matej Novak [:matej] from comment #4)
> Also, is it standard to have "Add-on" capitalized in cases like this? In
> marketing copy we only capitalize it when it directly follows Firefox, which
> is why I'm asking.

In the add-ons manager it is capitalized most of the time. see about:addons
Conforming to your convention would require to have it not capitalized in this dropdown menu as it is not about the brand. If this is what you suggest, we should file another bug to update that.

Comment 6

[Matej Novak [:matej]]

(In reply to Markus Jaritz [:maritz] (UX) from comment #5)
> (In reply to Matej Novak [:matej] from comment #4)
> > Also, is it standard to have "Add-on" capitalized in cases like this? In
> > marketing copy we only capitalize it when it directly follows Firefox, which
> > is why I'm asking.
> 
> In the add-ons manager it is capitalized most of the time. see about:addons
> Conforming to your convention would require to have it not capitalized in
> this dropdown menu as it is not about the brand. If this is what you
> suggest, we should file another bug to update that.

Nope, all good. Just wanted to make sure we were being consistent there. Thanks.

Comment 7

[Tony Mechelynck [:tonymec]]

(In reply to Markus Jaritz [:maritz] (UX) from comment #5)
> (In reply to Matej Novak [:matej] from comment #4)
> > Also, is it standard to have "Add-on" capitalized in cases like this? In
> > marketing copy we only capitalize it when it directly follows Firefox, which
> > is why I'm asking.
> 
> In the add-ons manager it is capitalized most of the time. see about:addons
> Conforming to your convention would require to have it not capitalized in
> this dropdown menu as it is not about the brand. If this is what you
> suggest, we should file another bug to update that.

In the drop-down menu all menuitems which include the word have it capitalized: "Add-on" or "Add-ons"; OTOH the label in the search box just right of it has "Search all add-ons" in lowercase. I suppose developers have more urgent things to worry about than this, though.

Comment 8

[Alexandre Poirot [:ochameau]]

So as I stated in bug 1185460, I'm willing to move forward with this and contribute with patches if that helps.

I already highlighted the new "about:debugging" page in Firefox.
This new page, instroduced by Devtools is meant to be a usual page for developers that need to debug stuff that aren't just a specific webpage:
 - shared/service workers,
 - addons,
 - apps,
All also allow to debug all these contexts, plus tabs for remote targets like:
 - firefox devices,
 - FxOS simulator addon (=FxOS desktop runtime)
 - fennec / firefox for android
 - and also chrome/ios tabs via "Valence" addon

We can add a UI in about:addons while trying to ensure it won't mislead regular Firefox users. I think Markus is right in comment 3, most users don't even go to about:addons. So adding something slightly hidden in a sub menu should work.
But I think we also have to add a UI to about:debugging as it fully targets developers. I opened bug 1221141 for this.

Comment 9

[Andy McKay]

I don't know the timeline or plans for about:debugging is there anything outlining that? We are looking for something that can land and be usable as soon as possible, so adding something to the about:addons menu seems a good way to go. Thanks for filing that other bug.

Comment 10

[Robert Helmer [:rhelmer]]

(In reply to Matej Novak [:matej] from comment #4)
> "Install Add-on temporarily from File..."

1) since it is possible to install an add-on temporarily from both a directory and a file, is it worth trying any distinction here or should we stick with "File" in the label above?

2) do we want to display any kind of confirmation dialog as we do with the existing "Install Add-on from File..."?

Comment 11

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #10)
> (In reply to Matej Novak [:matej] from comment #4)
> > "Install Add-on temporarily from File..."
> 
> 1) since it is possible to install an add-on temporarily from both a
> directory and a file, is it worth trying any distinction here or should we
> stick with "File" in the label above?
> 
> 2) do we want to display any kind of confirmation dialog as we do with the
> existing "Install Add-on from File..."?

3) Related to #1 - our filepicker implementation does not seem to be able to select a directory/folder or a file, in the same dialog.

I think we'll need to have both "Install Add-on temporarily from File..." and "Install Add-on temporarily from Folder...". The former allows selecting multiple files but the latter does not allow selecting multiple folders.

Comment 12

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: Bug 1209344 - remove debug button from about:addons r?mossop

Bug 1209344 - about:addons UI for temporarily-installed add-ons f?mossop

Comment 13

[Robert Helmer [:rhelmer]]

Comment on attachment 8695626 [details]
MozReview Request: Bug 1209344 - remove debug button from about:addons r?mossop

Wondering what you think of this approach - the main thing that's confusing here is that our platform-specific nsIFilePicker implementations (e.g. https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsFilePicker.mm but same for GTK and Windows) do not allow for selecting both files and folders in the same file picker. MDN used to imply that they did, I just corrected https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIFilePicker and asked for technical review, just in case I am missing something here.

Furthermore it's only possible to multi-select files, but not folders.

So - this patch has a separate menu item for installing from File or from Folder. Note that the code is largely identical, I can refactor that to be shared if you agree with this approach.

Also just a quick question on tests - the only test I can find for the current "Install Add-on from File..." is https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/browser/browser_bug567127.js - should I make a new test, or should I rename that one and test this here? I expect there to be a lot of shared code between the two, not sure it's worth having a whole separate file, but I could rename it to something more generic without the bug # perhaps?

Comment 14

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #13)
> Comment on attachment 8695626 [details]
> MozReview Request: Bug 1209344 - about:addons UI for temporarily-installed
> add-ons f?mossop
> 
> Wondering what you think of this approach - the main thing that's confusing
> here is that our platform-specific nsIFilePicker implementations (e.g.
> https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsFilePicker.mm
> but same for GTK and Windows) do not allow for selecting both files and
> folders in the same file picker. MDN used to imply that they did, I just
> corrected
> https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/
> Interface/nsIFilePicker and asked for technical review, just in case I am
> missing something here.
> 
> Furthermore it's only possible to multi-select files, but not folders.
> 
> So - this patch has a separate menu item for installing from File or from
> Folder. Note that the code is largely identical, I can refactor that to be
> shared if you agree with this approach.
> 
> Also just a quick question on tests - the only test I can find for the
> current "Install Add-on from File..." is
> https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> test/browser/browser_bug567127.js - should I make a new test, or should I
> rename that one and test this here? I expect there to be a lot of shared
> code between the two, not sure it's worth having a whole separate file, but
> I could rename it to something more generic without the bug # perhaps?

Oh I should also mention - there is no confirmation dialog currently, I am not sure it's desired but if so then I noticed that the current "Install Add-on from File..." code re-uses webInstaller.onWebInstallRequested() which we'll need to adjust (it requires AddonManager Install objects, which we don't have).

If a confirmation dialog is desired then I am sure we can figure out the right way to expose that, I just didn't want to do the work if it's not necessary :)

Comment 15

[Robert Helmer [:rhelmer]]

Attachment: screen shot of current UI in attachment 8695626

This is a screenshot of the dialog on OS X 10.11.1 with attachment 8695626 [details] (fx-team repo).

Comment 16

[Robert Helmer [:rhelmer]]

I don't think this depends on bug 1185460, please re-instate if I'm mistaken.

Comment 17

[Robert Helmer [:rhelmer]]

Andy and I have been discussing whether we need this, now that there is about:debugging and it supports this. It has some of the issues that I brought up earlier in this bug, so I filed bug 1231129 which will allow it to load from unpacked dirs as well as files.

Do we still want this feature in about:addons, given that about:debugging exists?

Comment 18

[Kris Maglione [:kmag]]

I think it's a question of how discoverable about:debugging is, and how likely add-on developers are to look for this feature there. So, my questions are:

How are developers going to find out about about:debugging?

Are they mostly going to be looking for this feature when they're debugging (or when they're thinking about what they're doing as debugging)?

Are we going to continue to surface other debugging features in about:addons without this one? If so, are they going to know/remember they need to go somewhere else to get to this functionality?

Are most developers going to automatically open the add-on manager when they want to install an extension, and get frustrated when this functionality isn't there? If so, what are we going to do to guide them to the correct solution?

Comment 19

[Kris Maglione [:kmag]]

Also, I strongly suspect that, when loading from a folder, most developers aren't going to want the add-on to be temporary. They're probably going to be loading it that way because they work on it regularly, and want changes to be picked up with minimal extra effort. Only offering this for temporary add-ons is probably going to be frustrating.

Comment 20

[Robert Helmer [:rhelmer]]

(In reply to Kris Maglione [:kmag] from comment #19)
> Also, I strongly suspect that, when loading from a folder, most developers
> aren't going to want the add-on to be temporary. They're probably going to
> be loading it that way because they work on it regularly, and want changes
> to be picked up with minimal extra effort. Only offering this for temporary
> add-ons is probably going to be frustrating.

This is a good point - I think it is covered by bug 1185460.

Comment 21

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #20)
> (In reply to Kris Maglione [:kmag] from comment #19)
> > Also, I strongly suspect that, when loading from a folder, most developers
> > aren't going to want the add-on to be temporary. They're probably going to
> > be loading it that way because they work on it regularly, and want changes
> > to be picked up with minimal extra effort. Only offering this for temporary
> > add-ons is probably going to be frustrating.
> 
> This is a good point - I think it is covered by bug 1185460.

Hm actually looks like the title on that changed - I am not sure it's about being able to load an unpacked directory with signing enabled anymore, given that.

We should file a separate bug for this issue IMHO.

Comment 22

[Robert Helmer [:rhelmer]]

(In reply to Kris Maglione [:kmag] from comment #18)
> I think it's a question of how discoverable about:debugging is, and how
> likely add-on developers are to look for this feature there. So, my
> questions are:
> 
> How are developers going to find out about about:debugging?
> 
> Are they mostly going to be looking for this feature when they're debugging
> (or when they're thinking about what they're doing as debugging)?
> 
> Are we going to continue to surface other debugging features in about:addons
> without this one? If so, are they going to know/remember they need to go
> somewhere else to get to this functionality?
> 
> Are most developers going to automatically open the add-on manager when they
> want to install an extension, and get frustrated when this functionality
> isn't there? If so, what are we going to do to guide them to the correct
> solution?

I don't know the answers to these questions, but a few folks have mentioned that it would be nice to move developer-focused features out of about:addons to devtools (e.g. about:debugging).

I don't mind doing this in about:addons and the current patch I put up for feedback WFM, but if we're just going to move it out of about:addons later then it'd be less confusing for add-on developers to decide that now.

Comment 23

[Kris Maglione [:kmag]]

(In reply to Robert Helmer [:rhelmer] from comment #22)
> I don't know the answers to these questions, but a few folks have mentioned
> that it would be nice to move developer-focused features out of about:addons
> to devtools (e.g. about:debugging).
> 
> I don't mind doing this in about:addons and the current patch I put up for
> feedback WFM, but if we're just going to move it out of about:addons later
> then it'd be less confusing for add-on developers to decide that now.

Yeah, that's fair. Either way, I'd rather be consistent, so if we don't add this to about:addons, it would be nice to remove the Debug buttons, and add something to point developers to about:debugging, as soon as possible.

Comment 24

[Dave Townsend [:mossop]]

(In reply to Robert Helmer [:rhelmer] from comment #14)
> (In reply to Robert Helmer [:rhelmer] from comment #13)
> > Comment on attachment 8695626 [details]
> > MozReview Request: Bug 1209344 - about:addons UI for temporarily-installed
> > add-ons f?mossop
> > 
> > Wondering what you think of this approach - the main thing that's confusing
> > here is that our platform-specific nsIFilePicker implementations (e.g.
> > https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsFilePicker.mm
> > but same for GTK and Windows) do not allow for selecting both files and
> > folders in the same file picker. MDN used to imply that they did, I just
> > corrected
> > https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/
> > Interface/nsIFilePicker and asked for technical review, just in case I am
> > missing something here.
> > 
> > Furthermore it's only possible to multi-select files, but not folders.
> > 
> > So - this patch has a separate menu item for installing from File or from
> > Folder. Note that the code is largely identical, I can refactor that to be
> > shared if you agree with this approach.
> > 
> > Also just a quick question on tests - the only test I can find for the
> > current "Install Add-on from File..." is
> > https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> > test/browser/browser_bug567127.js - should I make a new test, or should I
> > rename that one and test this here? I expect there to be a lot of shared
> > code between the two, not sure it's worth having a whole separate file, but
> > I could rename it to something more generic without the bug # perhaps?
> 
> Oh I should also mention - there is no confirmation dialog currently, I am
> not sure it's desired but if so then I noticed that the current "Install
> Add-on from File..." code re-uses webInstaller.onWebInstallRequested() which
> we'll need to adjust (it requires AddonManager Install objects, which we
> don't have).
> 
> If a confirmation dialog is desired then I am sure we can figure out the
> right way to expose that, I just didn't want to do the work if it's not
> necessary :)

I think if we're putting this in about:debugging then we don't need confirmation, we should probably display a message saying that the add-on has been installed and include the add-on's name to confirm they got the right one.

Comment 25

[Robert Helmer [:rhelmer]]

(In reply to Kris Maglione [:kmag] from comment #23)
> (In reply to Robert Helmer [:rhelmer] from comment #22)
> > I don't know the answers to these questions, but a few folks have mentioned
> > that it would be nice to move developer-focused features out of about:addons
> > to devtools (e.g. about:debugging).
> > 
> > I don't mind doing this in about:addons and the current patch I put up for
> > feedback WFM, but if we're just going to move it out of about:addons later
> > then it'd be less confusing for add-on developers to decide that now.
> 
> Yeah, that's fair. Either way, I'd rather be consistent, so if we don't add
> this to about:addons, it would be nice to remove the Debug buttons, and add
> something to point developers to about:debugging, as soon as possible.

OK after chatting w/ Dave and Andy and devtools folks as well, I've morphed this bug to be about removing the debug buttons from about:addons and linking to about:debugging.

Comment 26

[Robert Helmer [:rhelmer]]

(In reply to Dave Townsend [:mossop] from comment #24)
> (In reply to Robert Helmer [:rhelmer] from comment #14)
> > (In reply to Robert Helmer [:rhelmer] from comment #13)
> > > Comment on attachment 8695626 [details]
> > > MozReview Request: Bug 1209344 - about:addons UI for temporarily-installed
> > > add-ons f?mossop
> > > 
> > > Wondering what you think of this approach - the main thing that's confusing
> > > here is that our platform-specific nsIFilePicker implementations (e.g.
> > > https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsFilePicker.mm
> > > but same for GTK and Windows) do not allow for selecting both files and
> > > folders in the same file picker. MDN used to imply that they did, I just
> > > corrected
> > > https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/
> > > Interface/nsIFilePicker and asked for technical review, just in case I am
> > > missing something here.
> > > 
> > > Furthermore it's only possible to multi-select files, but not folders.
> > > 
> > > So - this patch has a separate menu item for installing from File or from
> > > Folder. Note that the code is largely identical, I can refactor that to be
> > > shared if you agree with this approach.
> > > 
> > > Also just a quick question on tests - the only test I can find for the
> > > current "Install Add-on from File..." is
> > > https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> > > test/browser/browser_bug567127.js - should I make a new test, or should I
> > > rename that one and test this here? I expect there to be a lot of shared
> > > code between the two, not sure it's worth having a whole separate file, but
> > > I could rename it to something more generic without the bug # perhaps?
> > 
> > Oh I should also mention - there is no confirmation dialog currently, I am
> > not sure it's desired but if so then I noticed that the current "Install
> > Add-on from File..." code re-uses webInstaller.onWebInstallRequested() which
> > we'll need to adjust (it requires AddonManager Install objects, which we
> > don't have).
> > 
> > If a confirmation dialog is desired then I am sure we can figure out the
> > right way to expose that, I just didn't want to do the work if it's not
> > necessary :)
> 
> I think if we're putting this in about:debugging then we don't need
> confirmation, we should probably display a message saying that the add-on
> has been installed and include the add-on's name to confirm they got the
> right one.

Comment 27

[Robert Helmer [:rhelmer]]

Comment on attachment 8695626 [details]
MozReview Request: Bug 1209344 - remove debug button from about:addons r?mossop

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

Comment 28

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: Bug 1209344 - link to about:debugging from about:addons r?mossop

Bug 1209344 - link to about:debugging from about:addons r?mossop

Comment 29

[Robert Helmer [:rhelmer]]

Attachment: screen shot of UI in attachment 8698196

Here's a screen shot of attachment 8698196 [details] - this adds a "Debug Add-ons" command in the "Gear" button pull-down menu in about:addons. The other patch removes "Debug" buttons from about:addons.

The "Debug Add-ons" command opens about:debugging#addons (or focuses the tab if already open).

Comment 30

[Dave Townsend [:mossop]]

Comment on attachment 8695626 [details]
MozReview Request: Bug 1209344 - remove debug button from about:addons r?mossop

https://reviewboard.mozilla.org/r/27159/#review25083

Comment 31

[Dave Townsend [:mossop]]

Comment on attachment 8698196 [details]
MozReview Request: Bug 1209344 - link to about:debugging from about:addons r?mossop

https://reviewboard.mozilla.org/r/27851/#review25085

::: toolkit/mozapps/extensions/test/browser/browser_debug_addons.js:22
(Diff revision 1)
> +      });

Looks like you've indented slightly too much here

Comment 32

[Robert Helmer [:rhelmer]]

Comment on attachment 8695626 [details]
MozReview Request: Bug 1209344 - remove debug button from about:addons r?mossop

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

Comment 33

[Robert Helmer [:rhelmer]]

Comment on attachment 8698196 [details]
MozReview Request: Bug 1209344 - link to about:debugging from about:addons r?mossop

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

Comment 34

[Robert Helmer [:rhelmer]]

Comment on attachment 8695626 [details]
MozReview Request: Bug 1209344 - remove debug button from about:addons r?mossop

https://reviewboard.mozilla.org/r/27159/#review25101

Comment 35

[Robert Helmer [:rhelmer]]

Comment on attachment 8698196 [details]
MozReview Request: Bug 1209344 - link to about:debugging from about:addons r?mossop

https://reviewboard.mozilla.org/r/27851/#review25105

Comment 36

[Robert Helmer [:rhelmer]]

(In reply to Dave Townsend [:mossop] from comment #31)
> Comment on attachment 8698196 [details]
> MozReview Request: Bug 1209344 - link to about:debugging from about:addons
> r=mossop
> 
> https://reviewboard.mozilla.org/r/27851/#review25085
> 
> ::: toolkit/mozapps/extensions/test/browser/browser_debug_addons.js:22
> (Diff revision 1)
> > +      });
> 
> Looks like you've indented slightly too much here

Can't wait until we enable the eslint style rules :)

Comment 37

[Robert Helmer [:rhelmer]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=025afe11453f

Comment 38

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7a5367ac58a4
https://hg.mozilla.org/integration/fx-team/rev/a1393f2f306e

Comment 39

[Carsten Book [:Tomcat]]

sorry backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=6216881&repo=fx-team

Comment 40

[Carsten Book [:Tomcat]]

also https://treeherder.mozilla.org/logviewer.html#?job_id=6216880&repo=fx-team could be related (not sure about that one)

Comment 41

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/dd4bb6fad8da
https://hg.mozilla.org/integration/fx-team/rev/a42ef45dc721

Comment 42

[Robert Helmer [:rhelmer]]

(In reply to Carsten Book [:Tomcat] from comment #39)
> sorry backed out for test failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=6216881&repo=fx-team

Oops I should've caught that in the try run - thanks!

Comment 43

[Robert Helmer [:rhelmer]]

Comment on attachment 8695626 [details]
MozReview Request: Bug 1209344 - remove debug button from about:addons r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27159/diff/3-4/

Comment 44

[Robert Helmer [:rhelmer]]

Comment on attachment 8698196 [details]
MozReview Request: Bug 1209344 - link to about:debugging from about:addons r?mossop

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

Comment 45

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #42)
> (In reply to Carsten Book [:Tomcat] from comment #39)
> > sorry backed out for test failures like
> > https://treeherder.mozilla.org/logviewer.html#?job_id=6216881&repo=fx-team
> 
> Oops I should've caught that in the try run - thanks!

The reason this happened is that I made a mistake in the "try:" syntax and it didn't run the tests that I intended it to.

The failing test now WFM locally (I had removed a bit of code that about:debugging still depends on from XPIProviders.jsm), also pushing to tryserver w/ the correct syntax now to make sure there isn't anything else.

Comment 46

[Robert Helmer [:rhelmer]]

Comment on attachment 8698196 [details]
MozReview Request: Bug 1209344 - link to about:debugging from about:addons r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27851/diff/3-4/

Comment 47

[Robert Helmer [:rhelmer]]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ac14b1af757&selectedJob=15100725

I don't *think* any of the failures here are related to this patch, so this is ready for review.

Comment 48

[Dave Townsend [:mossop]]

https://reviewboard.mozilla.org/r/27157/#review26539

::: toolkit/mozapps/extensions/test/browser/browser-common.ini
(Diff revision 5)
> -[browser_debug_button.js]

Please remove the test file too.

Comment 49

[Robert Helmer [:rhelmer]]

Comment on attachment 8695626 [details]
MozReview Request: Bug 1209344 - remove debug button from about:addons r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27159/diff/4-5/

Comment 50

[Robert Helmer [:rhelmer]]

Comment on attachment 8698196 [details]
MozReview Request: Bug 1209344 - link to about:debugging from about:addons r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27851/diff/4-5/

Comment 51

[Robert Helmer [:rhelmer]]

https://reviewboard.mozilla.org/r/27159/#review25103

Comment 52

[Robert Helmer [:rhelmer]]

Setting checkin-needed since I can't use autoland at the moment (bug 1237380)

Comment 53

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/330e05d5ee79
https://hg.mozilla.org/integration/fx-team/rev/9cb535d7ea7e

Comment 54

[Nigel Babu [:nigelb]]

https://hg.mozilla.org/integration/fx-team/rev/1982e6826fb8ee4ce1840f239982c870cafa9c92
Bug 1209344 - Touch CLOBBER so random build failures are fixed

Comment 55

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/330e05d5ee79
https://hg.mozilla.org/mozilla-central/rev/9cb535d7ea7e
https://hg.mozilla.org/mozilla-central/rev/1982e6826fb8

Comment 56

[Will Bamberg [:wbamberg]]

This is done: https://developer.mozilla.org/en-US/Add-ons/Add-on_Debugger#Opening_the_Add-on_Debugger

Comment 57

[CosminB]

This issue is verified as fixed on Firefox 51.0a1 (2016-09-14) on Windows 7 64-bit.

The “Debug” buttons are now displayed only in about:debugging and the "Debug Add-ons" command from the “Gear” button pull-down menu in about:addons opens about:debugging#addons (or focuses the tab if already open).