509080 - [E-2.1.1] Thunderbird Parity

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Collector Extension, defect, P3)

Description

[Justin Scott [:fligtar]]

The Add-on Collector should work in Thunderbird the same as it does in Firefox.

If it is too difficult to get working in Thunderbird 2, please let me know.

Comment 1

[Brian King [:kinger]]

I have this working now, mostly. Should I check in?

Comment 2

[Brian King [:kinger]]

Attachment: [checked in] Initial Patch

Here is the initial patch.

Comment 3

[Brian King [:kinger]]

Attachment: In action in TB 3b3

Here is what it looks like (Vista).

I have not tested it much, and there are a couple of TODDs, namely:

- first run experience
- redirect web page loads to nsIExternalProtocolService.loadURI

Comment 4

[Justin Scott [:fligtar]]

Sure.

Comment 5

[Brian King [:kinger]]

Comment on attachment 394264 [details] [diff] [review]
[checked in] Initial Patch

Patch checked in ... still some work to do here.

Feedback welcome.

Comment 6

[Fred Wenzel [:wenzel]]

Is the fact that I read this bug title as "Thunderbird Party" a sign that it's Friday?

Comment 7

[Brian King [:kinger]]

Anyone have any ideas for a first-run experience here?

Comment 8

[Brian King [:kinger]]

(In reply to comment #3)
> - redirect web page loads to nsIExternalProtocolService.loadURI

Done, r49325.

Another TODO:
- toolbar button

Comment 9

[Ken Saunders]

After installing (and restarting), the add-ons manager is presented as normal and from there most users will explore the options of their new add-on(s). When doing so for the Add-on Collector (Firefox and Thunderbird), the first tab shown is the Manage Subscriptions one and options throughout the Add-on Collector Settings window are disabled (for obvious reasons) so perhaps the Log in (General Settings tab) option should be presented first or moved to the Manage Subscriptions tab? 
Of course a user can/may always just click on the Subscriptions tab in the add-ons manager, but again, most people will go to an add-on's options first especially when the Extensions tab is shown first after installing an add-on.

Comment 10

[Ken Saunders]

Attachment: Log in Option in first tab

Comment 11

[Brian King [:kinger]]

I think just moving General Settings to be the first panel would be the best solution.

Comment 12

[Ken Saunders]

Auto-publisher is not working for me.

All Auto-publisher settings checked (except Listed in public Collection Directory)
13 installed/enabled add-ons 

Tried:
Disabling 1 add-on not hosted on AMO (Mail Tweak)
Disabled all but Add-on Collector and Mr Tech Toolkit
Tried an AMO account with no collections
Tried yelling at 
Tried kicking it (not recommended) 

The throbber just spins and spins and...
Left it running for about 5 minutes at one point

Any suggestions?

bandwagon-2008-08-18.xpi
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.1) Gecko/20090715 Thunderbird/3.0b3

Comment 13

[Ken Saunders]

Thought that changing this would help, but no go.

extensions.bandwagon.amo_host addons.mozilla.org

QA/Execution/Web Testing/AMO/5.0.9 Collector Test Plan
Setup: First, change extensions.bandwagon.amo_host from "addons.mozilla.org" to "preview.addons.mozilla.org"

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Ken - try changing extensions.bandwagon.debug to true, and getting output via the Error Console; probably needs a new bug.

What's the build date/version of the collector, again?  bandwagon-2008-08-18.xpi doesn't sound right.

The latest I was using is https://people.mozilla.org/~jscott/bandwagon/2009-08-11.xpi

Comment 15

[Ken Saunders]

The one that you have doesn't appear to include Thunderbird.

    <em:targetApplication>
      <Description>
        <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id> <!-- firefox -->
        <em:minVersion>3.0</em:minVersion>
        <em:maxVersion>3.5.*</em:maxVersion>
      </Description>
    </em:targetApplication>


bandwagon-2008-08-18.xpi
     <!-- Firefox -->
    <em:targetApplication>
      <Description>
        <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id>
        <em:minVersion>3.0</em:minVersion>
        <em:maxVersion>3.5.*</em:maxVersion>
      </Description>
    </em:targetApplication>

    <!-- Thunderbird -->
    <em:targetApplication>
      <Description>
        <em:id>{3550f703-e582-4d05-9a08-453d09bdfdc6}</em:id>
        <em:minVersion>3.0b3</em:minVersion>
        <em:maxVersion>3.0b4pre</em:maxVersion>
      </Description>
    </em:targetApplication>

Comment 16

[Brian King [:kinger]]

@stephend - I sent Ken a more recent build to do some TB testing. If you want one, just ping me or fligtar. The latest code is in SVN.

Comment 17

[Brian King [:kinger]]

(In reply to comment #12)
> Auto-publisher is not working for me.

Thanks ... I can reproduce...

FMI:

Error: Bandwagon.Model.Collection is not a constructor
Source File: chrome://bandwagon/content/ui/settingsController.js
Line: 539

Comment 18

[Brian King [:kinger]]

I fixed that issue (r49548) and sent a new test XPI out to Ken.

Comment 19

[Ken Saunders]

Thanks Brian. The Auto-publisher is working as expected.

I'm guessing that the Manage button is something new. It doesn't seem to be functional (yet?).

I'm sure that ya'll are aware but I thought that I'd ask about it anyway. "Add to Firefox" (button) is currently displayed for Thunderbird add-ons when viewing an add-on in a collection. Are you still a ways off from providing the functionality to add Thunderbird add-ons to Thunderbird from the Add-on Collector?

Also, is this the most current test plan to follow (in addition of course to generally kicking the tires)?
https://wiki.mozilla.org/QA/Execution/Web_Testing/AMO/5.0.9_Collector_Test_Plan

Finally (then you can go and enjoy a cold frosty beverage), does it currently make any difference if extensions.bandwagon.amo_host value is set to addons.mozilla.org?

Thanks

Comment 20

[Brian King [:kinger]]

(In reply to comment #19)
> I'm guessing that the Manage button is something new. It doesn't seem to be
> functional (yet?).

Correct. That is bug 507328 which should be fixed soon.
 
> I'm sure that ya'll are aware but I thought that I'd ask about it anyway. "Add
> to Firefox" (button) is currently displayed for Thunderbird add-ons when
> viewing an add-on in a collection. Are you still a ways off from providing the
> functionality to add Thunderbird add-ons to Thunderbird from the Add-on
> Collector?

Good catch! I'll make sure to fix that and review the other strings to ensure no other Firefox specific ones are in there

> Also, is this the most current test plan to follow (in addition of course to
> generally kicking the tires)?
> https://wiki.mozilla.org/QA/Execution/Web_Testing/AMO/5.0.9_Collector_Test_Plan

That is the latest I believe, yes.

> Finally (then you can go and enjoy a cold frosty beverage), does it currently
> make any difference if extensions.bandwagon.amo_host value is set to
> addons.mozilla.org?

What do you mean? Do you have it set to something else?

Comment 21

[Ken Saunders]

(In reply to comment #20)

> > Finally (then you can go and enjoy a cold frosty beverage), does it currently
> > make any difference if extensions.bandwagon.amo_host value is set to
> > addons.mozilla.org?
> 
> What do you mean? Do you have it set to something else?

No, I was referring to this.
https://wiki.mozilla.org/QA/Execution/Web_Testing/AMO/5.0.9_Collector_Test_Plan
Setup: First, change extensions.bandwagon.amo_host from "addons.mozilla.org" to "preview.addons.mozilla.org"

Comment 22

[Brian King [:kinger]]

(In reply to comment #21)
> No, I was referring to this.
> https://wiki.mozilla.org/QA/Execution/Web_Testing/AMO/5.0.9_Collector_Test_Plan
> Setup: First, change extensions.bandwagon.amo_host from "addons.mozilla.org" to
> "preview.addons.mozilla.org"

That is just running against different server code, that perhaps has new features that are not in the production. I suggest just sticking with production for testing right now ... i.e. addons.mozilla.org

Comment 23

[Brian King [:kinger]]

The only remaining issue here that I am aware of is whether we should support Thunderbird 2. We had a similar discussion about supporting Firefox 2, which uses the same extension manager code. I can't find a log of that discussion anywhere though (checked BZ and email).

IIRC, we would need to fork some XUL and UI controller JS code, as the UI hooks are considerably different. Most of the back-end would be the same, except for some 1.9 only interfaces that we could possibly be using.

For Firefox 2 we were not compelled to support it because the majority of users had moved to 3. This is not the case with TB2 -> TB3.

I don't have an accurate sense for how long it would take, but a good guesstimate would be a days work.

Justin, any thoughts?

Comment 24

[Justin Scott [:fligtar]]

Per the original description of this bug, I was assuming we were going to support TB2 unless told otherwise. As TB3 is not yet released, I would like to see if we could try TB2 support, but only after the other work like Fennec support.

Comment 25

[Brian King [:kinger]]

TB2 Update:

I've made some good progress, and I'd say we are 90% parity with TB3/FF3.x.

The relevant commits, for reference, are:

r51969
r51974

Some serious issues remain:

1) Expanded add-ons do not always show when clicked. They flash up and then disappear. Here is a collection where none show correctly when expanded:
https://addons.mozilla.org/en-US/firefox/collections/view/d3189cee-ddd5-d4b5-3122-938ed3eeb2b6
Here is a collection where they all show, but with layout issues:
https://addons.mozilla.org/en-US/firefox/collections/view/d7b5de88-dc94-748b-ea64-37ae3d3be45a
2) Styling for selected collections on left and selected add-ons on right does not apply.
3) A login prompt for the API comes up from time to time. You can trigger it by doing a manual reload of subscriptions.
3) On Windows, the Settings window is over-sized and has some extra icon content on left.
4) 'Get Themes' or 'Get Extensions' link appears on login screen.
5) Toolbar button is the wrong size on some platforms.
6) Plural forms don't work, so I have put a hard-coded string hack in for now as a band-aid. See the dateAdded property in bandwagon.xml.
7) Clicking the toolbar button does not open the Add-ons window on the Subscriptions panel.

Dave, can you please look at issues 1-3. Feel free to break out new bugs if they are protracted issues. I'll do the same for some others.

And now for some gory details about the port:

- The Add-ons window overlay is forked (in chrome.manifest). extensionsOverlay2.xul applies to TB2.
- A new overlay (extensionsBoxOverlay.xul) containing the subscriptions container and content was created. It is used by extensionsOverlay.xul and extensionsOverlay2.xul.
- 'override' does not work in chrome.manifest in TB2. So I put versions of the stylesheets we override directly in skin/ for now so that TB2 picks them up. These means there are no platform specific versions for now. We may not need them, because the TB theme in 2 is similar across platforms, but there are still sizing issues (the Mac toolbar icons are larger than Windows)

Comment 26

[David McNamara [:mackers]]

(In reply to comment #25)
> Dave, can you please look at issues 1-3. Feel free to break out new bugs if
> they are protracted issues. I'll do the same for some others.

Ok. I'll take a look. (Up to the first number 3!)

> - The Add-ons window overlay is forked (in chrome.manifest).
> extensionsOverlay2.xul applies to TB2.

Just a niggle, but would this overlay be better called something like extensionsOverlayTB2.xul (I'm all for lucidity in naming conventions).

Comment 27

[David McNamara [:mackers]]

(In reply to comment #25)
> 1) Expanded add-ons do not always show when clicked. They flash up and then
> disappear. 

Appear to be a layout glitch in TB2. Expanded add-ons do not display if the description 'is not the right length'. Added some code to pad with nbsp, but it's hacky.

> 3) A login prompt for the API comes up from time to time. You can trigger it by
> doing a manual reload of subscriptions.

This is serious. TB2 ignores mozBackgroundRequest = true on XMLHttpRequest, causing the auth dialog to popout. I'm not sure there's a workaround for this.

Comment 28

[Brian King [:kinger]]

(In reply to comment #26)
> Just a niggle, but would this overlay be better called something like
> extensionsOverlayTB2.xul (I'm all for lucidity in naming conventions).

Sure.

Comment 29

[Brian King [:kinger]]

(In reply to comment #27)
> Appear to be a layout glitch in TB2. Expanded add-ons do not display if the
> description 'is not the right length'. Added some code to pad with nbsp, but
> it's hacky.

Right, I don't think this is the right fix. I don't even see any change with this fix. I bet the problem it in the XUL/XBL. I'll have a deeper look.
 
> This is serious. TB2 ignores mozBackgroundRequest = true on XMLHttpRequest,
> causing the auth dialog to popout. I'm not sure there's a workaround for this.

I've poked around and have not been able to find an alternative in FF2. What exactly is happening here? Are we sending login credentials in a request, and still we are being asked for authentication? Can you see another way of doing it?

Comment 30

[Brian King [:kinger]]

On a related note, the add-on now gets stuck on the login screen in TB2. You might have to log out to reproduce it. I think this is related to the new token feature that was added to login recently. 

In _invalidateExtensionsDeck in collectionsPaneController.js, bandwagonService.isAuthenticated() is always returning false.

Comment 31

[David McNamara [:mackers]]

(In reply to comment #29)
> I've poked around and have not been able to find an alternative in FF2. What
> exactly is happening here? Are we sending login credentials in a request, and
> still we are being asked for authentication? Can you see another way of doing
> it?

To get the URL of the API call to authenticate, we first need to get the service document URL. When we request the service document URL, the API is (expectedly) returning a standard "Unauthorized" HTTP response code, so TB is popping up an authentication dialog.

If we hard code the URL of the authentication call, then we could get around this. But it would break the way we're supposed to use the API (i.e. only knowing the URL of the service document).

(In reply to comment #29)
> Right, I don't think this is the right fix. I don't even see any change with
> this fix. I bet the problem it in the XUL/XBL. I'll have a deeper look.

Me too. The problem is with the xul:description element that we put the add-on description in. Removing that might solve the problem. We could use a label for TB2. Crippled, I know, but maybe a valid choice for supported an old and buggy platform.

(In reply to comment #30)
> On a related note, the add-on now gets stuck on the login screen in TB2. You
> might have to log out to reproduce it. 

This is a symptom of the API authentication dialog problem above.

Comment 32

[Brian King [:kinger]]

Moving meta bug out of release target.