Closed Bug 799944 Opened 9 years ago Closed 9 years ago

New Localizations From L10N Sprints

Categories

(Mozilla Localizations :: vi / Vietnamese, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: arky, Unassigned)

References

Details

Attachments

(2 files, 2 obsolete files)

Patch with new localization strings from recent L10N sprints.
Attachment #669957 - Flags: review?(community)
Comment on attachment 669957 [details] [diff] [review]
Patch with new localizations strings

:l10n can't do reviews, that's not a person. I'd put that r? on me, if I was confident that I can get to it soon, but I'm not sure I'll have time today, and then I'm pto for two weeks. No good suggestions.
Attachment #669957 - Flags: review?(community) → review?
Comment on attachment 669957 [details] [diff] [review]
Patch with new localizations strings

Can you review and commit this patch for Vi L10N team.
Attachment #669957 - Flags: review? → review?(milos)
Thanks Pike, Doing r? for Milos then
Milos Can you please help us review this.
Yang, Please review the region.properties and other system files that should not be changed. Perhaps we need to fix these problems using the local hg repo.
Yes.  Please tell me or point me to the location where I can find the review criteria for this patch.
Arky, Duong,

Is this initial landing or just a patch for adding new strings? If former, then your(my) comment 5 in this bug doesn't really apply here.
(In reply to Dương H. Nguyễn from comment #6)
> Yes.  Please tell me or point me to the location where I can find the review
> criteria for this patch.

Yang, the files arky mentions should remain in English. Simply revert those files back to their source state without Vietnamese strings.
Milos, We are trying to add new strings. The Vietnamese L10N is stuck Fx6 for now. Can you please review the patches and land them before Yang gets his commit access.
Comment on attachment 669957 [details] [diff] [review]
Patch with new localizations strings

Review of attachment 669957 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the late response here. I'd love to see a new patch based on the feedback below.

This patch doesn't cleanly apply to aurora at this point anymore.

The changes to browser/chrome/browser-region/region.properties should be undone. I don't think they're the right thing to do, and won't work out as they are either. If you want to make changes beyond the license change, please file a separate bug, and explain why you want to make which change.

Below are the errors that compare-locales finds, and those are concerning a bit. I'd like to understand why the translations of Reset turned out as they are. Right now, I'm in the situation that I found one occurence 'cause it triggered a translation check, but I couldn't tell other places. It'd be great if a native speaker could review an updated patch for similar issues. That is, if we can't obviously point a finger on why this happened just for that one string.

Given that the patch doesn't apply, changes region.properties, and introduces errors, I'm giving this an r-.

::: browser/chrome/browser/browser.dtd
@@ +518,5 @@
>  <!ENTITY fullZoomReduceCmd.accesskey    "T">
>  <!ENTITY fullZoomReduceCmd.commandkey   "-">
>  <!ENTITY fullZoomReduceCmd.commandkey2  "">
>  
> +<!ENTITY fullZoomResetCmd.label         "Phóng 100%">

You need to use an xml entity, % per se can't be used in DTDs.

You'll need to encode that with &#037;

This shows up more often, I think. compare-locales finds those and reports then as "unexpected parser state", not really helpful without this explanation, I know.

::: browser/chrome/browser/preferences/sync.dtd
@@ +10,5 @@
>  <!-- The page shown when logged in... -->
>  
>  <!-- Login error feedback -->
>  <!ENTITY updatePass.label             "Cập nhật">
> +<!ENTITY resetPass.label              "Phóng 100%">

Encode the % again, though this looks like bad translation memory for "Reset".

::: dom/chrome/layout/HtmlForm.properties
@@ +4,2 @@
>  
> +Reset=Phóng 100%

Once I searched for '100%', I found this one, too.

::: security/manager/chrome/pippki/pippki.dtd
@@ +11,5 @@
>  <!ENTITY setPassword.meter.label "Äá» an toàn mật khẩu">
>  <!ENTITY setPassword.meter.loading "Äang tải">
>  
>  <!-- Values for resetpassword.xul -->
> +<!ENTITY resetPasswordButtonLabel "Phóng 100%">

The % had to be encoded again, but this is also bad translation memory for Reset?

::: toolkit/chrome/global/config.dtd
@@ +36,5 @@
>  <!ENTITY modify.label "Sá»­a Äá»i">
>  <!ENTITY modify.accesskey "S">
>  <!ENTITY toggle.label "Bật-Tắt">
>  <!ENTITY toggle.accesskey "B">
> +<!ENTITY reset.label "Phóng 100%">

Again, %, but more so, bad translation memory?
Attachment #669957 - Flags: review?(milos) → review-
Thank you Axel, Much appreciated.  

Yang, Can you please review the suggestions and create a new patch.
I'm back!  Really sorry for the late response.  Thank you Axel for the thorough review. I'm inspecting the patch and will create a new one based on your suggestions.
Attached patch Vi L10n patch after review (obsolete) — Splinter Review
Re-send patch after fixing errors that Axel pointed out.
Comment on attachment 686635 [details] [diff] [review]
Vi L10n patch after review

Review of attachment 686635 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the update, but I'll need to r- this again.

Please provide a patch against http://hg.mozilla.org/releases/l10n/mozilla-aurora/vi/, this patch doesn't apply again. Make sure to hg pull and hg update before adding your modifications.

Also, browser/chrome/browser-region/region.properties shouldn't change at all.

There are still issues with "Phóng 100%", all but one are wrong translations, see below.

::: browser/chrome/browser/preferences/sync.dtd
@@ +10,5 @@
>  <!-- The page shown when logged in... -->
>  
>  <!-- Login error feedback -->
>  <!ENTITY updatePass.label             "Cập nhật">
> +<!ENTITY resetPass.label              "Phóng 100&#037">

This is a translation mistake, I'm really sure.

::: dom/chrome/layout/HtmlForm.properties
@@ +4,2 @@
>  
> +Reset=Phóng 100%

... and here

::: security/manager/chrome/pippki/pippki.dtd
@@ +11,5 @@
>  <!ENTITY setPassword.meter.label "Äá» an toàn mật khẩu">
>  <!ENTITY setPassword.meter.loading "Äang tải">
>  
>  <!-- Values for resetpassword.xul -->
> +<!ENTITY resetPasswordButtonLabel "Phóng 100&#037">

... same here.

::: toolkit/chrome/global/config.dtd
@@ +36,5 @@
>  <!ENTITY modify.label "Sá»­a Äá»i">
>  <!ENTITY modify.accesskey "S">
>  <!ENTITY toggle.label "Bật-Tắt">
>  <!ENTITY toggle.accesskey "B">
> +<!ENTITY reset.label "Phóng 100&#037">

... and here
Attachment #686635 - Flags: review-
My apologies, Axel, I cloned the wrong repo :-(.  Thank you for your comments, I'll fix it right away.
Attached patch New L10n vi patch (obsolete) — Splinter Review
Here you are the new patch
Attachment #686635 - Attachment is obsolete: true
Duong, you still have changes to region.properties file, which, as Axel said, shouldn't be in this patch. If you do want to make changes to this file, please file a separate bug, describe the changes you want to do and elaborate them, and upon agreement on said changes with one of l10n-drivers, create a new patch solely for the changes you want in region.properties.
Thank you Milos.  I missed Axel's comment on that.  I'll file a bug about region.properties later.  This is the patch which excludes the file.
Attachment #687482 - Attachment is obsolete: true
Attachment #687828 - Flags: review?(l10n)
Attachment #687828 - Flags: review?(l10n) → review-
Oops. Sorry I think I landed a whole lot of changes from the Narro to Pootle migration, didn't realise this was oustanding.  Not sure if we want to revert in this case?
I don't think we need a back-out yet, I followed up in an email.
I think we can close this bug now. Vietnamese is already in beta.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.