Last Comment Bug 763471 - Convert Dropbox Filelink provider code to addon
: Convert Dropbox Filelink provider code to addon
Status: RESOLVED FIXED
: dev-doc-needed
Product: Thunderbird
Classification: Client Software
Component: FileLink (show other bugs)
: 16 Branch
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Mike Conley (:mconley) - (needinfo me!)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-11 06:45 PDT by Jb Piacentino
Modified: 2012-09-26 06:49 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+


Attachments
Remove Dropbox from comm-central - v1 (60.49 KB, patch)
2012-06-11 14:03 PDT, Mike Conley (:mconley) - (needinfo me!)
no flags Details | Diff | Splinter Review
First version of Dropbox add-on (17.82 KB, application/x-xpinstall)
2012-06-11 14:04 PDT, Mike Conley (:mconley) - (needinfo me!)
no flags Details
Another iteration of the Dropbox add-on (14.29 KB, application/x-xpinstall)
2012-06-12 08:34 PDT, Mike Conley (:mconley) - (needinfo me!)
no flags Details
Patch v2 [checked-in] (69.03 KB, patch)
2012-06-13 14:15 PDT, Mike Conley (:mconley) - (needinfo me!)
mozilla: review+
standard8: approval‑comm‑aurora-
standard8: approval‑comm‑beta-
Details | Diff | Splinter Review

Description Jb Piacentino 2012-06-11 06:45:32 PDT
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) - (needinfo me!) 2012-06-11 14:03:58 PDT
Created attachment 632016 [details] [diff] [review]
Remove Dropbox from comm-central - v1

This patch removes the Dropbox component from comm-central.
Comment 2 Mike Conley (:mconley) - (needinfo me!) 2012-06-11 14:04:56 PDT
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
Comment 3 Jb Piacentino 2012-06-12 00:43:18 PDT
What is the plan for localization ?
Comment 4 Ludovic Hirlimann [:Usul] 2012-06-12 00:53:18 PDT
(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) 2012-06-12 01:22:26 PDT
(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) - (needinfo me!) 2012-06-12 06:30:54 PDT
(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) - (needinfo me!) 2012-06-12 06:35:52 PDT
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] 2012-06-12 06:39:48 PDT
(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) - (needinfo me!) 2012-06-12 08:34:05 PDT
Created attachment 632266 [details]
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) - (needinfo me!) 2012-06-13 08:24:14 PDT
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 2012-06-13 13:12:44 PDT
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) - (needinfo me!) 2012-06-13 13:17:53 PDT
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) - (needinfo me!) 2012-06-13 13:50:47 PDT
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) - (needinfo me!) 2012-06-13 14:15:39 PDT
Created attachment 632878 [details] [diff] [review]
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 2012-06-14 10:08:17 PDT
Comment on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

builds and mozmill tests pass with this.
Comment 16 Mike Conley (:mconley) - (needinfo me!) 2012-06-14 10:14:03 PDT
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) - (needinfo me!) 2012-06-14 10:15:06 PDT
comm-central: https://hg.mozilla.org/comm-central/rev/13e86c34c73f
Comment 18 David :Bienvenu 2012-06-14 10:20:58 PDT
(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) - (needinfo me!) 2012-06-14 10:22:56 PDT
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 2012-06-14 10:29:26 PDT
(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) - (needinfo me!) 2012-06-14 11:00:35 PDT
Comment on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

Alright, let's go for it.
Comment 22 Mark Banner (:standard8) 2012-06-18 02:58:53 PDT
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) 2012-06-21 04:18:05 PDT
Tracking for 15 because of the string change/disabling.
Comment 24 Onno Ekker [:nONoNonO UTC+1] 2012-06-23 03:33:37 PDT
Oops, sorry for that. Too bad I can't change the tracking flag bag to +...
Comment 25 Mike Conley (:mconley) - (needinfo me!) 2012-09-26 06:49:57 PDT
The add-on has been released:

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

Note You need to log in before you can comment on or make changes to this bug.