Closed Bug 461805 Opened 17 years ago Closed 16 years ago

Add-ons with broken locales should not be hosted or should warn the author

Categories

(addons.mozilla.org Graveyard :: Developer Pages, enhancement)

Product:
addons.mozilla.org Graveyard
Component:
Developer Pages
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: underpass_bugzilla, Unassigned)

Details

User-Agent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.9.0.3) Gecko/2008092416 Firefox/3.0.3 Build Identifier: It is possible for an author to upload extensions with broken locales (i.e. incomplete locales); it can happen for example if some strings are added to the .dtd or the .properties file and the changes are not made accordingly to all the locales contained in the XPI file. Reproducible: Always Actual Results: An incomplete .dtd or .properties file in a locale directory inside the XPI file could make the addon break a profile giving XUL errors to a user of a particular language. Not all the users can figure out what's happening and how to correct this kind of problem. Expected Results: If an extension has an incomplete locale (i.e. a .dtd or a .properties file that do not match those in the en-US locale) then it happens that either: 1) the author is clearly warned that an incomplete locale does exist and it can damage a user profile, or 2) the author is automatically prevented from uploading such a file, or 3) the incomplete locale is stripped from the file making it impossible to damage any user profile Maybe other choices are possible.
There are already controls that prevent authors from uploading extensions on AMO (for example checks on the maxversion parameter). Since we already have a tool to check locale integrity (compare-locale), maybe AMO should run this tool and add a "pass compare-locale" as a mandatory step.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Component: Policy → Developer Pages
Ever confirmed: true
QA Contact: policy → developers
Simon, do you know of any particular offending add-ons?
The last example that I can remember is the update to ForecastFox in june (probably this one https://addons.mozilla.org/it/firefox/addons/versions/398#version-0.9.7.5) Popular extension->automatic update->lots of broken Firefox and dozens of requests on the support forum.
http://adblockplus.org/blog/managing-locales AdBlockPlus has a script for checking locales that we should try to modify and use when reviewing extensions. IMO if there is a broken locale, a new extension should be kept in the sandbox, and a public one demoted.
I've added a section to the reviewing guide that will be filled out once we reach some decent conclusions here. https://wiki.mozilla.org/Update:Editors/ReviewingGuide#Step_6._Locale_Testing cc'ing Axel for possible recommendations for tools/scripts to hook into.
The script I use for Adblock Plus makes a bunch of assumptions that aren't generic. However, I also wrote a generic script for TomTom that has a subset of the functionality - testing that the files are valid UTF-8, testings DTD and properties files syntax. I'll discuss getting this script released under some open source license.
I strongly recommend not going for Wladimir's scripts. They're based on code that hasn't gotten any bugfixes for over a year, while other codes got significant improvements over time. Either https://developer.mozilla.org/En/Compare-locales or http://diary.braniecki.net/tag/silme/ (unstable as of now) are the right basis. There's http://babelwiki.babelzilla.org/index.php?title=GoofyPlan_for_an_extension_testing_machine, too.
Axel, as I already wrote in a reply to your blog comment - you are mistaken that my script is based on compare-locales.pl, it is unrelated. However, I agree that this script is anything but generic, it is a quick and dirty solution and is very specific to Adblock Plus. I never claimed otherwise.
TomTom's locale testing script is now under MPL: http://hg.mozdev.org/adblockplus/file/tip/third-party/test-locale-files.pl This script is way simpler and much more generic. It tests UTF-8, DTD files and properties syntax. It also happens to accept a larger subset of the DTD syntax because it doesn't need to extract the data out of DTD files (I doubt that any extension will use syntax features not covered there).
(In reply to comment #9) > TomTom's locale testing script is now under MPL: > http://hg.mozdev.org/adblockplus/file/tip/third-party/test-locale-files.pl > > This script is way simpler and much more generic. It tests UTF-8, DTD files and > properties syntax. It also happens to accept a larger subset of the DTD syntax > because it doesn't need to extract the data out of DTD files (I doubt that any > extension will use syntax features not covered there). Does it work with XPI files to avoid manual unpacking? Is it something that could live server side, i.e. when the XPI is uploaded to AMO?
Do we need anything beside of comparing two directories to check if there are any missing strings? If yes, then silme's dummy compare-locales is doing exactly that: http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/18b385b27f68/scripts/compare-locales.py And could be a solid generic base for such script.
Hrm. Wladimir's code is not going to get approved by l10n-drivers. I don't really bother what he's using locally, but for a Mozilla service, it's not acceptable. I won't waste time on an in-depth review, just simply glancing at the parsing code clearly states that it's lightyears away from the state of the art. I don't enjoy bashing other folks code like this, but that's the way it is right here. http://hg.mozilla.org/users/axel_mozilla.com/tooling/log/4f0aed56efbe/mozilla/testing/tests/l10n/lib/Mozilla/Parser.py is just a fraction of the history it's missing.
(In reply to comment #11) > Do we need anything beside of comparing two directories to check if there are > any missing strings? It depends. For example I'm thinking about files with wrong encoding (not UTF-8): does Silme's parser fail in this situation?
yes. you can easily report if file encoding is not utf8.
actually: silem.io.file.getSource can try several encodings or use the one that you offer (utf-8) and throw an exception if it wont work.
We need something that: 1. works directly on XPI/JAR (if it's possible) or unpack the files in a temp folder 2. finds the en-US locale folder 3. compare the en-US locale with all other locales 4. give a fail/pass result I see at least two problems here: * is en-US required to upload extensions on AMO ? If not, we don't have a fixed reference locale * how do we select which locales we have to check? Navigating folders or parsing chrome.manifest/install.rdf?
1) Silme has an IO abstraction level. Usually you'll use silme.io.file, but you can use silme.io.jar instead to get the same files. This way you don't have to unpack zip. 3) that's what silme.diff.* package is for
(In reply to comment #12) > I won't waste time on an in-depth review, just simply glancing at the parsing > code clearly states that it's lightyears away from the state of the art. I > don't enjoy bashing other folks code like this, but that's the way it is right > here. Axel, I don't mind you being in favor of a different solution, there are many reasons why you would want it. However, that's a moot point. The parsing code is a 1:1 translation of the BNF in the XML specification (http://www.w3.org/TR/REC-xml/#dt-doctype), as is your parser.
This is going to be integrated into the 5.0.9 push: all_l10n_checkCompleteness in validation.php, line 619: http://viewvc.svn.mozilla.org/vc/addons/trunk/site/app/controllers/components/validation.php?view=markup
5.0.9 has landed, just like RJ so magically predicted.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 5.0.9
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.