Closed
Bug 661083
Opened 13 years ago
Closed 10 years ago
Let developers localize add-on metadata with cfx
Categories
(Add-on SDK Graveyard :: General, enhancement, P1)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: KWierso, Unassigned)
References
Details
Attachments
(6 files)
It'd be nice if developers had a way to specify localized add-on metadata (like name, description, etc) directly from package.json without having to manually edit install.rdf in the generated XPI files.
This is a work in progress patch to try to add this.
The patch adds an em:localized element (and child elements) to the app-extension copy of install.rdf, which is used as the SDK's template when creating an extension's XPI file.
In packaging.py, the 'localized' property is added so that it is recognized in package.json.
The rest of the patch is in rdf.py:
It makes a tiny change to the remove() method, making sure that the "localized" versions of elements don't get deleted when the unlocalized versions are. (I'm not sure if this is really necessary.)
It adds a "localize()" method, which takes in the package.json attributes and adds them to install.rdf correctly.
It takes the em:localized element that was added to app-extension/install.rdf up above, and inserts a copy of it back into the working copy of the rdf file (because there can be more than one localization, I couldn't just add the properties directly to the template). One copy gets attached for every localization specified in package.json.
One a copy has been attached, it looks at each of the child elements and checks for a corresponding property in package.json. If a property exists in there, that property is inserted into the rdf file. If that property doesn't exist, that child element is removed.
After all of the locales have been added, the template version of em:localized gets removed.
I'd like some feedback on how this is being done.
As it stands, the patch works completely (as far as I can tell, at least. haven't tried running any tests). Installing the generated XPI picks up the localizations for the name, description and other metadata.
It still needs to add to the package specification documentation to show the new stuff, but I don't want to do that until the format of the new stuff gets more finalized.
(The generated install.rdf doesn't look very pretty, though. Open to suggestions!)
Attachment #536518 -
Flags: feedback?(myk)
Reporter | ||
Comment 1•13 years ago
|
||
This is the sample package.json contents that I used to test this.
Reporter | ||
Comment 2•13 years ago
|
||
And here's what the generated install.rdf looks like from that sample.
Reporter | ||
Comment 3•13 years ago
|
||
Right now, for convenience (to me) the patch uses the rdf element names for the new package.json property names, not the names already in package.json. (All it has to do is strip off the "em:" and then the names match up.)
The final version of this should probably use the same names as the rest of package.json.
Reporter | ||
Comment 4•13 years ago
|
||
Comment on attachment 536521 [details]
generated install.rdf
Changing the attached install.rdf's mimetype so you can see the not-so-pretty output easier.
Attachment #536521 -
Attachment mime type: application/rdf+xml → text/plain
Reporter | ||
Updated•13 years ago
|
Severity: normal → enhancement
Comment 5•13 years ago
|
||
Our vision for addon localization is that addon developers shouldn't have to do any work to solicit and package localizations. Rather, they should just use strings in their addon (identifying them as needing localization as appropriate) and submit their addon to AMO for distribution, at which point AMO should automatically solicit localizations from localizers and package the localizations that localizers contribute.
So I don't want to add functionality that enables addon developers to manually provide localizations. Rather, we should make sure that the project to provide a "common pool" service for addon localization includes the ability to localize metadata in addition to strings embedded in code.
cc:ing gandalf so he's aware of this additional requirement for the common pool localization service (which he may already be aware of!).
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
Updated•13 years ago
|
Attachment #536518 -
Flags: feedback?(myk)
Comment 6•12 years ago
|
||
Reopening this as we didn't got resources to release commonpool.
We can now implement this through existing properties file.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Reporter | ||
Updated•12 years ago
|
Priority: -- → P1
Comment 7•11 years ago
|
||
Until this is fixed, can you add a hint to the documentation that the name and description cannot be localized using the l10n mechanisms provided by the addon-sdk? It is bad enough to have to unzip the xpi, manually edit install.rdf and zip it again. But I, (and others) have spent hours trying to figure this out because no one assumes that this most basic functionallity is actually missing.
https://addons.mozilla.org/en-US/developers/docs/sdk/latest/dev-guide/tutorials/l10n.html
Thx!
Updated•11 years ago
|
Flags: needinfo?(evold)
Comment 8•11 years ago
|
||
Sorry for the delay in getting to this. I think Alex suggests in (comment 6) that we provide these localizations via the properties files using the method described here https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localizing_extension_descriptions.
This would help us achieve what Myk describes in comment 5 too I believe.
Flags: needinfo?(evold)
Updated•11 years ago
|
Depends on: native-jetpack
Comment 9•10 years ago
|
||
Looks good! we would need some tests here tho, we should probably just patch jpm to handle this use case though, we are going to try to terminate cfx.
https://github.com/mozilla/jpm
Sorry we didn't see this sooner :(
Attachment #8457645 -
Flags: review-
Comment 10•10 years ago
|
||
> Looks good! we would need some tests here tho, we should probably just patch
> jpm to handle this use case though, we are going to try to terminate cfx.
Eric, if it looks good, just approve it. We developers are waiting for a fix almost three years and now there are two fixes. The one suggested here by Wes Kocher and the new one by kukushechkin (which I personally prefer); that last one is just waiting for approval. I don't care if cfx has a feature or if some tool named jpm will solve the problem one day (maybe in another 3 years). We developers need solutions to problems and we don't need them the day after tomorrow but the day before yesterday. Most of the time people here complain that nobody writes code, well, we have code, now it all fails because somebody fails to approve it.
Comment 11•10 years ago
|
||
(In reply to TGOS from comment #10)
> > Looks good! we would need some tests here tho, we should probably just patch
> > jpm to handle this use case though, we are going to try to terminate cfx.
>
> Eric, if it looks good, just approve it. We developers are waiting for a fix
> almost three years and now there are two fixes. The one suggested here by
> Wes Kocher and the new one by kukushechkin (which I personally prefer); that
> last one is just waiting for approval. I don't care if cfx has a feature or
> if some tool named jpm will solve the problem one day (maybe in another 3
> years). We developers need solutions to problems and we don't need them the
> day after tomorrow but the day before yesterday. Most of the time people
> here complain that nobody writes code, well, we have code, now it all fails
> because somebody fails to approve it.
As a rule all changes must be accompanied by tests, this is important as it stops us from accidentally regressing functionality in the future. If a feature is important enough to fix then it is important enough that we shouldn't unintentionally break it. If you're willing to provide test code then we can land this change, otherwise it will have to wait for one of the team to do so.
Comment 12•10 years ago
|
||
Okay, I re-implemented the fix, because I didn't like that the original fix used install.rdf names for localization keys instead of packages.json names (e.g. it used "addon_creator" instead of "addon_author"). Also the original fix broke two other unit tests, which I had to fix as well. Other than that my fix works pretty much the same. A unit test which tests for localized output in the install.rdf file is included.
https://github.com/mozilla/addon-sdk/pull/1594
I think not too bad for the first Python code I ever wrote myself.
Flags: needinfo?(evold)
Flags: needinfo?(dtownsend+bugmail)
Comment 13•10 years ago
|
||
Up to Erik whether he wants to re-review the code for cfx
Attachment #8475332 -
Flags: review?(evold)
Flags: needinfo?(evold)
Flags: needinfo?(dtownsend+bugmail)
Comment 14•10 years ago
|
||
Comment on attachment 8475332 [details]
new patch
So the format that we want to support is defined here https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localizing_extension_descriptions#Localizing_before_Gecko_1.9
"addon_name" for example should be "extensions.EXTENSION_ID.name" and there needs to be a preference set like `pref("extensions.EXTENSION_ID.name", "PATH_TO_LOCALIZATION_FILE");`
Attachment #8475332 -
Flags: review?(evold) → review-
Comment 15•10 years ago
|
||
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #14)
> So the format that we want to support is defined here
> https://developer.mozilla.org/en-US/docs/Mozilla/Localization/
> Localizing_extension_descriptions#Localizing_before_Gecko_1.9
Who is "we"? What you've been linking is the Gecko pre-1.9 method. Gecko 1.9 is Firefox 3 and SDK based add-ons don't support Firefox 2, so they always target a Gecko version for sure 1.9 or later.
To quote from that same page:
>> Gecko 1.9 includes a new, more robust method for localizing add-on
>> descriptions and other metadata. All of the different descriptions
>> now appear in the install.rdf file using em:localized properties.
>> Each has at least one em:locale property marking the locale that
>> the information should be used for and then all the various strings
>> for the locale.
So why exactly do you want to support a legacy localization scheme which I have not seen in use for ages at any existing add-on and that is only required for Firefox version that cannot even run SKD based add-ons produced by the cfx tool?
Despite the fact that it is pretty annoying to write string keys with the "extension ID" (which may change a couple of times during early development), this scheme uses the install.rdf field names, which do not match the package.json field names (which is even more annoying to most developers, especially add-on beginners that know package.json but have no idea what install.rdf is):
package.json -> install.rdf
---------------------------
title -> name
author -> creator
description -> description
homepage -> homepageURL
So why not using the "more robust" method, that add-on developers are using today, unfortunately by unpacking the XPI created by cfx, making the changes by hand and repacking it? (see comment 7)
First you complain no unit test but you said the code looks good. Now there is a unit test but all of a sudden the code that looked good before is wrong. If I do it exactly as you described, you'd find another excuse. Geeez, this bug is from 2011! You force us developers through this repacking hell for 3 years already, just because you have a great vision. People also want a working solution for add-ons hosted on their own webpages or elsewhere in the web, and they want be able to test localization locally, so a solution that relies on some automatic AMO localization service is no solution at all IMHO, because it is no generic solution suitable for everyone. Despite this bug is named "Let developers localize add-on metadata.", *developers* and not some unknown people using some AMO localization service.
Honestly, I give it up. Coding for Mozilla is the biggest waste of time ever. I've done it in the past for Firefox, many years ago and instead of taking my working code every user was happy with, the code was rejected out of reasons I'd call nitpicking and the bug has never been fixed up to today. From now on I will maintain my private cfx fork, implement the features I've been missing forever (I have some others, e.g. <em:targetPlatform> support) and keep using this one for add-on development. Alternatively, I may write a cfx-wrapper/post-processor, that later on "fixes" the generated XPIs (automatically unpack, modify and repack the content); advantage would be that it could also fix XPIs produced by jpm that will probably lack the same set of fundamental features as cfx. I'm sick waiting decades for the simplest bugs getting fixed and the simplest features getting implemented.
If anybody comes here and needs localization support for the install.rdf today, just download the current official cfx release, apply my patch to it (I can also provide a patched branch on request for download at GitHub) and use it. It works great, you will love it. No XPI repacking any longer. Don't wait for this bug to be fixed, it won't be fixed within the next three years either.
Comment 16•10 years ago
|
||
Attachment #8476295 -
Flags: review?(rFobic)
Updated•10 years ago
|
Attachment #8476295 -
Flags: review?(rFobic) → review+
Comment 17•10 years ago
|
||
Commits pushed to master at https://github.com/mozilla/addon-sdk
https://github.com/mozilla/addon-sdk/commit/b55b79799c6fb2ab4c554578b7f73e7534247808
Fix for bug 661083: "Let developers localize add-on metadata."
https://bugzilla.mozilla.org/show_bug.cgi?id=661083
The package.json keys "title", "author", "description", and "homepage"
can be localized using the property file keys "addon_title",
"addon_author", "addon_description", and "addon_homepage".
Conflicts:
python-lib/cuddlefish/rdf.py
https://github.com/mozilla/addon-sdk/commit/f1e469cd2765c4f00e97f9222dffc8cccda5f961
Bug 661083 using "extensions.<EXTENSION_ID>." instead of "addon_" as a prefix for key names
https://github.com/mozilla/addon-sdk/commit/7ba6834d74a413d50e902026cc568647c334b453
Bug 661083 adding a testaddon for the manifest localization
https://github.com/mozilla/addon-sdk/commit/2acf356359a5c311ba8255b4dff584b4d1fbb851
Merge pull request #1597 from erikvold/661083
Bug 661083 manifest localization r=@gozala
Updated•10 years ago
|
Status: REOPENED → RESOLVED
Closed: 13 years ago → 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Blocks: cfx.py
Summary: Let developers localize add-on metadata. → Let developers localize add-on metadata with cfx
Comment 18•10 years ago
|
||
Sorry we won't be releasing any new versions of cfx, jpm is the replacement https://www.npmjs.com/package/jpm
I've reverted this change in https://github.com/mozilla/addon-sdk/commit/0ccba4c256ada078cb2eff8fd77a8543b5a3ee87 so that this bug does not delay native-jetpacks (bug 915376)
Bug 1123428 has been opened for add-on manifest localization support for jpm.
Resolution: FIXED → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•