763471 - Convert Dropbox Filelink provider code to addon

Status: RESOLVED FIXED

(Thunderbird :: FileLink, defect)

Description

[Jb Piacentino]

Let's review the options and timing so we can release it fully localized.
TB15 or 16 seem a good target.

This is also a good opportunity to show how to make a Filelink addon.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Remove Dropbox from comm-central - v1

This patch removes the Dropbox component from comm-central.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: First version of Dropbox add-on

This add-on puts Dropbox support back into Thunderbird.

Source code posted here: https://github.com/mikeconley/thunderbird-filelink-dropbox

Comment 3

[Jb Piacentino]

What is the plan for localization ?

Comment 4

[Ludovic Hirlimann [:Usul]]

(In reply to Mike Conley (:mconley) from comment #2)
> Created attachment 632017 [details]
> First version of Dropbox add-on
> 
> This add-on puts Dropbox support back into Thunderbird.
> 
> Source code posted here:
> https://github.com/mikeconley/thunderbird-filelink-dropbox

Shouldn't we link to some mdn page instead of the support page in the projects description on github ?

Are we allowed to publish 
const kAppKey = "7xkhuze09iqkghm";
const kAppSecret = "3i5kwjkt74rkkjc";

Comment 5

[Mark Banner (:standard8)]

(In reply to Jb Piacentino from comment #3)
> What is the plan for localization ?

I would suggest:

1) Seed the add-on with l10n by obtaining the current localisations from the l10n repos, probably using the release or beta repositories.
2) Post the add-on on http://www.babelzilla.org/ to allow localisation work to take place in the future.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

(In reply to Ludovic Hirlimann [:Usul] from comment #4)
> (In reply to Mike Conley (:mconley) from comment #2)
> > Created attachment 632017 [details]
> > First version of Dropbox add-on
> > 
> > This add-on puts Dropbox support back into Thunderbird.
> > 
> > Source code posted here:
> > https://github.com/mikeconley/thunderbird-filelink-dropbox
> 
> Shouldn't we link to some mdn page instead of the support page in the
> projects description on github ?

How about I do both?

> 
> Are we allowed to publish 
> const kAppKey = "7xkhuze09iqkghm";
> const kAppSecret = "3i5kwjkt74rkkjc";

I asked Blake about this yesterday, and I think we're OK - it's not like these tokens are any *more* public being on Github than being on hg.mozilla.org. Unless there was some sort of contractual obligation to keep those tokens on hg.mozilla.org only, I don't think there's a problem here. Someone with more legal grounding in our contracts might correct me here.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Also, in case anybody is wondering, I spent a good chunk of yesterday trying to build this add-on using the Mozilla Add-on SDK. I got pretty far - I was able to get the component registered, the category entry created, all of the content pages viewable, etc. It fell over on l10n, since, according to the Add-on SDK documentation:

"There's no support for content scripts, HTML files, or CSS files: at the moment, you can only localize strings appearing in JavaScript files that can require() SDK modules."

There's some support for localizing HTML in the bleeding edge of the Add-on SDK, but using it broke other things, so I decided to punt and use the old-school add-on method.

Comment 8

[Ludovic Hirlimann [:Usul]]

(In reply to Mike Conley (:mconley) from comment #6)
> (In reply to Ludovic Hirlimann [:Usul] from comment #4)
> > (In reply to Mike Conley (:mconley) from comment #2)
> > > Created attachment 632017 [details]
> > > First version of Dropbox add-on
> > > 
> > > This add-on puts Dropbox support back into Thunderbird.
> > > 
> > > Source code posted here:
> > > https://github.com/mikeconley/thunderbird-filelink-dropbox
> > 
> > Shouldn't we link to some mdn page instead of the support page in the
> > projects description on github ?
> 
> How about I do both?

sure

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Attachment: Another iteration of the Dropbox add-on

* Grabbed l10n strings from mozilla-beta - I think a few are still incomplete, but (I believe) we support about ~40 locales now.
* Removed upgrade offer links
* General spruce up of install.rdf

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 632016 [details] [diff] [review]
Remove Dropbox from comm-central - v1

I think this is all of it. Did I miss anything?

Comment 11

[David :Bienvenu]

Comment on attachment 632016 [details] [diff] [review]
Remove Dropbox from comm-central - v1

I think you missed mail\locales\jar.mn

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Oh good call.  A grep shows that I've also forgotten to include nsDropbox.js in removed-files.in.

It also shows that the UbuntuOne management pane is erroneously relying on Dropbox's implementation:

http://mxr.mozilla.org/comm-central/source/mail/components/cloudfile/content/UbuntuOne/management.xhtml#16

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 632016 [details] [diff] [review]
Remove Dropbox from comm-central - v1

Withdrawing review until I fix those issues.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v2 [checked-in]

* Removes Dropbox entries from mail/locales/jar.mn
* Removes nsDropbox.js entry from package-manifest.in
* Creates management.js for UbuntuOne, and points its management.xhtml at it
* Fixes erroneous "Dropbox" string in UbuntuOne tests

Comment 15

[David :Bienvenu]

Comment on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

builds and mozmill tests pass with this.

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

Did we want this for Aurora too? We can probably skip removing it from beta, since the Dropbox components aren't registered anyhow.

Comment 17

[Mike Conley (:mconley) (:⚙️)]

comm-central: https://hg.mozilla.org/comm-central/rev/13e86c34c73f

Comment 18

[David :Bienvenu]

(In reply to Mike Conley (:mconley) from comment #16)
> Comment on attachment 632878 [details] [diff] [review]
> Patch v2 [checked-in]
> 
> Did we want this for Aurora too? We can probably skip removing it from beta,
> since the Dropbox components aren't registered anyhow.

the main reason to drop it from aurora that I can see (other that saving a little bit of code size) is that it might reduce the patch errors when trying to port trunk patches to aurora.

Comment 19

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

Good point. If that's the case, should we drop it from beta as well?

Comment 20

[David :Bienvenu]

(In reply to Mike Conley (:mconley) from comment #19)
> Comment on attachment 632878 [details] [diff] [review]
> Patch v2 [checked-in]
> 
> Good point. If that's the case, should we drop it from beta as well?

I doubt that we're going to add new providers to beta, but it can't hurt...

Comment 21

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

Alright, let's go for it.

Comment 22

[Mark Banner (:standard8)]

Comment on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

I'm not sure if this is going to apply on beta, as we've already done some disabling there.

In any case, we need a version that doesn't touch the string files - even though it is just removal, it is best not to touch that for L10n and just let the removal ride the train.

Comment 23

[Mark Banner (:standard8)]

Tracking for 15 because of the string change/disabling.

Comment 24

[Onno Ekker [:nONoNonO UTC+1]]

Oops, sorry for that. Too bad I can't change the tracking flag bag to +...

Comment 25

[Mike Conley (:mconley) (:⚙️)]

The add-on has been released:

https://addons.mozilla.org/en-US/thunderbird/addon/dropbox-for-filelink/