Created attachment 615699 [details] Pull request 398 While working on HTML localization, I noticed too strict error handling in plural-rules module. It was throwing exception that prevent localization to be working. We should only send warning messages and use simple fallback plural rule instead.
Assignee: nobody → poirot.alex
Maybe it'd be better for Pike or gandalf to do the review here?
Priority: -- → P1
I'm not sure it makes sense to wait for such review for this particular fix. It basically prevents localization module to be completely disabled when we can't find the related plural rule. What we want to get is a l10n team review for the original code touched by this patch, i.e. bug 719402. And it looks like the main question is how much do we trust unicode.org? Is following dataset correct? http://unicode.org/repos/cldr/trunk/common/supplemental/plurals.xml Here is code related to plural rules: https://github.com/mozilla/addon-sdk/blob/master/packages/addon-kit/lib/l10n.js#L43-65 https://github.com/mozilla/addon-sdk/blob/stabilization/packages/api-utils/lib/l10n/plural-rules.js https://github.com/mozilla/addon-sdk/blob/master/python-lib/plural-rules-generator.py
(In reply to Alexandre Poirot (:ochameau) from comment #2) > I'm not sure it makes sense to wait for such review for this particular fix. > It basically prevents localization module to be completely disabled when we > can't find the related plural rule. > > What we want to get is a l10n team review for the original code touched by > this patch, i.e. bug 719402. > And it looks like the main question is how much do we trust unicode.org? > Is following dataset correct? > http://unicode.org/repos/cldr/trunk/common/supplemental/plurals.xml > > Here is code related to plural rules: > https://github.com/mozilla/addon-sdk/blob/master/packages/addon-kit/lib/l10n. > js#L43-65 > https://github.com/mozilla/addon-sdk/blob/stabilization/packages/api-utils/ > lib/l10n/plural-rules.js > https://github.com/mozilla/addon-sdk/blob/master/python-lib/plural-rules- > generator.py Feel free to land with a=@gozala (a => authorized) and we can try to get review from l10n team later. If there will be major issues we'll be able to followup then.
Commit pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/cc9ffc99c6241a3411d9870a091c81ab53cf9bb9 Merge pull request #398 from ochameau/updated-plural-rules Bug 746137: Improve error handling in localization code (plural rules) . a=@gozala
Comment on attachment 615699 [details] Pull request 398 Closing review.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.