Closed Bug 635688 Opened 9 years ago Closed 9 years ago

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

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set

Tracking

(blocking-thunderbird5.0 alpha3+)

RESOLVED FIXED
Thunderbird 3.3a3
Tracking Status
blocking-thunderbird5.0 --- alpha3+

People

(Reporter: standard8, Assigned: squib)

References

()

Details

Attachments

(1 file, 2 obsolete files)

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.
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.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Attachment #514122 - Flags: review?(bugzilla)
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.
Attached patch Add missing code, and more tests (obsolete) — Splinter Review
This should fix things up. I updated the test to make sure that tab reuse code actually works right.
Attachment #514122 - Attachment is obsolete: true
Attachment #514122 - Flags: review?(bugzilla)
Attachment #514377 - Flags: review?(bugzilla)
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?
Attachment #514377 - Flags: review?(bugzilla) → review-
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...
(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.
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").
Attachment #514377 - Attachment is obsolete: true
Attachment #514664 - Flags: review?(bugzilla)
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.
Attachment #514664 - Flags: review?(bugzilla) → review+
Checked in: http://hg.mozilla.org/comm-central/rev/268e28378355

Thanks Jim.
Status: ASSIGNED → RESOLVED
blocking-thunderbird5.0: needed → alpha3+
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a3
You need to log in before you can comment on or make changes to this bug.