Improve error handling in localization code (plural rules)

RESOLVED FIXED in 1.8

Status

P1
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: ochameau, Assigned: ochameau)

Tracking

unspecified
x86_64
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
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.
Attachment #615699 - Flags: review?(rFobic)
(Assignee)

Updated

7 years ago
Assignee: nobody → poirot.alex
Blocks: 739249
Maybe it'd be better for Pike or gandalf to do the review here?
Priority: -- → P1
(Assignee)

Comment 2

7 years ago
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.

Comment 4

7 years ago
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.
Attachment #615699 - Flags: review?(rFobic)
(Assignee)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Target Milestone: --- → 1.8
You need to log in before you can comment on or make changes to this bug.