Closed Bug 763471 Opened 12 years ago Closed 12 years ago

Convert Dropbox Filelink provider code to addon

Categories

(Thunderbird :: FileLink, defect)

16 Branch
defect
Not set
normal

Tracking

(thunderbird15+)

RESOLVED FIXED
Tracking Status
thunderbird15 + ---

People

(Reporter: jb, Assigned: mconley)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files, 2 obsolete files)

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.
This patch removes the Dropbox component from comm-central.
Assignee: nobody → mconley
Attached file First version of Dropbox add-on (obsolete) —
This add-on puts Dropbox support back into Thunderbird.

Source code posted here: https://github.com/mikeconley/thunderbird-filelink-dropbox
What is the plan for localization ?
(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";
(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.
(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.
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.
(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
* 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
Attachment #632017 - Attachment is obsolete: true
Comment on attachment 632016 [details] [diff] [review]
Remove Dropbox from comm-central - v1

I think this is all of it. Did I miss anything?
Attachment #632016 - Flags: review?(dbienvenu)
Comment on attachment 632016 [details] [diff] [review]
Remove Dropbox from comm-central - v1

I think you missed mail\locales\jar.mn
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 on attachment 632016 [details] [diff] [review]
Remove Dropbox from comm-central - v1

Withdrawing review until I fix those issues.
Attachment #632016 - Flags: review?(dbienvenu)
* 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
Attachment #632016 - Attachment is obsolete: true
Attachment #632878 - Flags: review?(dbienvenu)
Comment on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

builds and mozmill tests pass with this.
Attachment #632878 - Flags: review?(dbienvenu) → review+
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.
Attachment #632878 - Attachment description: Patch v2 → Patch v2 [checked-in]
comm-central: https://hg.mozilla.org/comm-central/rev/13e86c34c73f
Whiteboard: [leave open to iterate on addon]
(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 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?
Attachment #632878 - Flags: approval-comm-aurora?
(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 on attachment 632878 [details] [diff] [review]
Patch v2 [checked-in]

Alright, let's go for it.
Attachment #632878 - Flags: approval-comm-beta?
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.
Attachment #632878 - Flags: approval-comm-beta?
Attachment #632878 - Flags: approval-comm-beta-
Attachment #632878 - Flags: approval-comm-aurora?
Attachment #632878 - Flags: approval-comm-aurora-
Assignee: mconley → nobody
Component: General → FileLink
Keywords: dev-doc-needed
QA Contact: general → filelink
Assignee: nobody → mconley
Tracking for 15 because of the string change/disabling.
Oops, sorry for that. Too bad I can't change the tracking flag bag to +...
The add-on has been released:

https://addons.mozilla.org/en-US/thunderbird/addon/dropbox-for-filelink/
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [leave open to iterate on addon]
You need to log in before you can comment on or make changes to this bug.