635688 - Update openAddonsMgr to take a add-on pane URI as argument and correctly open the add-ons Manager pane

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Mark Banner (:standard8)]

Thunderbird's openAddonsMgr function is meant to take the add-on pane to display as an argument, but currently ignores it.

We need to port the changes that Firefox did to BrowserOpenAddonsMgr.

This is needed so that certain calls will work, e.g. openAddonsMgr("addons://list/theme") should open the add-on manager at the themes pane, which we need for at least one notification.

Comment 1

[Jim Porter (:squib)]

Attachment: Port the changes from BrowserOpenAddonsMgr

This pretty much just copies the code from Firefox and adds a simple test for it. Maybe more tests would be good, but I'm not sure what really needs testing with the addons manager.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 514122 [details] [diff] [review]
Port the changes from BrowserOpenAddonsMgr

I think you've missed a bit after the ping/pong - the bit where you check to see if emWindow has been set.

Comment 3

[Jim Porter (:squib)]

Attachment: Add missing code, and more tests

This should fix things up. I updated the test to make sure that tab reuse code actually works right.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 514377 [details] [diff] [review]
Add missing code, and more tests

This looks better, but I've just been testing it and it doesn't quite work properly.

If the add-ons manager isn't open, then it opens, but it doesn't switch to the right view - you need to update the callers of openAddonsMgr to add "addons://list/" onto the front of what they pass in.

If the add-ons manager is already open, then I see:

Error: Invalid view: undefined
Source File: chrome://mozapps/content/extensions/extensions.js
Line: 590

Not quite sure what that is yet, maybe a side effect of the wrong thing being passed?

Comment 5

[Jim Porter (:squib)]

What are you clicking on to open the addon manager? I opened it via Tools > Addons and it seems to work fine. It looks like there was an issue with opening up the theme window after installing a theme though...

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #5)
> What are you clicking on to open the addon manager? I opened it via Tools >
> Addons and it seems to work fine. It looks like there was an issue with opening
> up the theme window after installing a theme though...

Ah, so install Personas Plus extension (after overriding compatibility), use its menus and dig down into a category and select the "nnnnn more in xyz..." - that opens a content tab. Select a persona, select wear it, and you'll get a notification bar up with an option to "Manage Themes". That's the button I was clicking.

Comment 7

[Jim Porter (:squib)]

Attachment: Fix "manage themes" button in notification bar

Alright, looks like we were talking about the same thing, which is good. That looks like the only place that needed updated (at least, nothing else comes up in a search for "openAddonsMgr").

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 514664 [details] [diff] [review]
Fix "manage themes" button in notification bar

That's better.

>+  let tab = mc.tabmail.currentTabInfo;
>+  assert_content_tab_has_url(tab, 'about:addons');
>+  wait_for_content_tab_load(tab);
>+  assert_true(content_tab_e(tab, 'category-themes').selected,
>+              "Themes category should be selected!");

I had an issue here when I was running the test on my machine (which was also building in the background). It was complaining that the first assert failed because the url was 'about:blank', my guess is that it was taking longer for the page to start loading because the machine was under load.

Therefore I've moved that assert_content_tab_has_url after the wait_for_content_tab_load in both instances.

As I've got that in my local tree, I'll check it in for you in a moment.

Comment 9

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/268e28378355

Thanks Jim.