[yosemite] Simplified Chinese shows up in Traditional Chinese mode when saving an image.

RESOLVED FIXED in Firefox 42

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jhao, Assigned: othree (MozTW))

Tracking

33 Branch
mozilla42
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(8 attachments, 12 obsolete attachments)

134.10 KB, image/jpeg
Details
92.52 KB, image/jpeg
Details
752.04 KB, image/png
Details
43.48 KB, image/png
Details
44.76 KB, image/png
Details
80.87 KB, image/jpeg
Details
651.71 KB, image/png
Details
9.34 KB, patch
glandium
: review+
Details | Diff | Splinter Review
(Reporter)

Description

3 years ago
Created attachment 8511644 [details]
Screenshot

The language was in Traditional Chinese, but when the user look at an image, right-clicked and chose "Save as", some fields in the dialog were in Simplified Chinese.
The dialog is a native one from the OS as far as i know.
Do you see the same behavior in other file dialogs and does it work in other OSX Applications like Safari ?
Flags: needinfo?(jhao)
(Reporter)

Comment 2

3 years ago
Created attachment 8511775 [details]
Comparison with Safari

The same problem doesn't happen on Safari.
Flags: needinfo?(jhao)
Component: General → Widget: Cocoa
Product: Firefox → Core
Please provide detailed steps to reproduce, using a specific image from a specific page.
(Reporter)

Comment 4

3 years ago
The user who encountered this bug says it happens to every image he wants to save.

1. In Traditional Chinese mode
2. Right click on any image
3. 圖片另存新檔 (Save Image As)
> 1. In Traditional Chinese mode

What does this mean, exactly?
(Reporter)

Comment 6

3 years ago
1. Open System Settings
2. Go to Language & Region
3. Click "+", adding Chinese (Traditional) to preferred languages
4. Drag Chinese (Traditional) to the first priority

My personal experience is that the Firefox menu and popup dialogs are still in English, not in Chinese (Traditional), is it normal? The user who reported this to me sees popup dialogs in Chinese (Simplified) every time he saves an image.
I presume that by "Traditional Chinese" you mean 繁體中文.

I tried your STR and can't reproduce the problem.  (Note that after step 4 you need to log out and back in.)  I strongly suspect that either the problem doesn't happen with all images, or that there's something peculiar about how the original reporter's computer is set up.

So far I've only tested on OS X 10.7.5.  I'll also try on other versions.

I tested on the default Mozilla home page, by right-clicking on the Google graphic and choosing "Save Image As".  I used en-US Firefox 33.0.1.  All the items in the system file picker were in "complex" (non-simplified) characters.

Jonathan, are you able to reproduce this bug yourself?
> My personal experience is that the Firefox menu and popup dialogs
> are still in English, not in Chinese (Traditional), is it normal?

For en-US Firefox I think that yes, it is.  I also saw this.
I can't reproduce this bug on OS X 10.9.5 or 10.10 either.

Unless you, Jonathan, can reproduce this bug and are able to teach me how to, we should close this WORKSFORME or INCOMPLETE.
(Reporter)

Comment 10

3 years ago
I'm sorry that I can't reproduce either. Anyway, thanks to Steven.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
Reopen this bug, as I can reproduce this bug in Firefox 33.1.1 with OS X 10.10.1, both in zh-TW locale, and images as well as web pages on mozilla.org and google.com were tested.

Beside this Bug, the messages in save file dialog are also displayed in zh-CN like what I filed Bug 1102121. 
(it's normal in 10.9 Mavericks)
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
See Also: → bug 1102121
I think Bug 1102121 can be marked as duplicate of the bug, the two bugs seems to be the same issue.

I will try to post the problem and a possible workaround again:

Steps to reproduce:

1. On **OS X 10.10**, set system language to Traditional Chinese (zh-TW, 繁體中文). Older versions did not have the issue. I can reproduce on both 10.10 and 10.10.1.
2. **Install zh-TW l10n build of Firefox**, so you will have a Firefox app bundle with `zh.lproj` in `Contents/Resources`. Due to this requirement, I guess that language pack won't work.
For Nightly, use latest-mozilla-central-l10n.
3. Open any file picker dialogs (any that will trigger NSIFilePikcer to open)

I believe it is a OS X behavior regression, and I have a verified workaround by renaming zh.lproj to zh-Hant.lproj.
Peter, please provide your own, detailed STR -- as detailed as Hsiao-Ting's.
(In reply to Steven Michaud from comment #13)
> Peter, please provide your own, detailed STR -- as detailed as Hsiao-Ting's.

OK, new Steps to reproduce:
1. Have a Mac running with Yosemite, and switch system locale to Traditional Chinese (zh-TW, 繁體中文)
2. Install zh-TW build of Firefox
3. Open Bugzilla and try to file a new bug
4. Go to Attachment: section and click the "瀏覽…" (Browse…) to add an attachment,
5. Check the messages displayed inside the file picker (compare with the screenshots attached to Bug 1102121)
6A. Open any webpage with image (e.g., http://mozilla.org/en-US), right click to image, click "圖片另存新檔…" (Save Image As…), and observe the dialog
6B. Open any webpage with link, right click to link, click "鏈結另存新檔…" (Save Link As…), and observe the dialog
6C. Open any webpage, right click to page, click "另存新檔…" (Save As…), and observe the dialog

All 6A,B,C produce same dialog with zh-CN translation. That's pretty same to what described in Comment 4, 6, Hsiao-Ting's, and STR in Bug 1102121. It seems the key to reproduce is you need a Mac in zh-TW locale, with zh-TW build of Firefox.
Duplicate of this bug: 1102121
I'm probably the one best suited to work on this bug -- I'm the only one in Mozilla (and possibly outside of Apple) who'll be able to reverse engineer what changed in Yosemite to cause this bug.  But I won't have time to work on this bug for quite a while -- at least not until after the Mozilla cooincident work week in December, and likely not until next year.

> I believe it is a OS X behavior regression, and I have a verified
> workaround by renaming zh.lproj to zh-Hant.lproj.

But maybe working around this bug won't require full-scale reverse engineering.  Maybe we can just have two copies of this file -- one named zh.lproj and one named zh-Hant.lproj.  Someone should check.

For this fix to be acceptable, it will need to work (and to not cause problems) on all major versions of OS X from 10.6 up.  So it will need to be tested there, rather thoroughly.
Another thing:

People should look at how other browsers handle this problem (if they do).  Safari is most likely to already have worked around it.  But you should also look at Chrome and Opera.  Try to reproduce this bug in those browsers, and look to see what *.lproj files they have.

Comment 18

3 years ago
According to Yu's comment, I check 'Contents/Resources' of Opera, Chrome, and Safari. All of them use 'zh_TW.lproj', but only Firefox uses 'zh.lproj'. 
I hope this information is helpful.
To change the .lproj name used for a locale during l10n repackaging the following will need to use the AB_CD form but I suspect (I am by no means sure) that a list of which locales should use AB_CD will need to be created.
http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/l10n.mk#118

Updated

3 years ago
Summary: Simplified Chinese shows up in Traditional Chinese mode when saving an image. → [yosemite] Simplified Chinese shows up in Traditional Chinese mode when saving an image.
(Reporter)

Updated

3 years ago
Assignee: nobody → jhao
(Reporter)

Comment 20

3 years ago
Created attachment 8541436 [details] [diff] [review]
Generate zh-TW.lproj instead of zh.lproj

Hi Steve,

Could you review this patch?

The changes in browser/app/Makefile.in are for building desktop in zh-TW.
The changes in toolkit/locales/l10n.mk are for repacking.

I have tested both methods on 10.10, and with this patch they both produce zh-TW.lproj.
Attachment #8541436 - Flags: review?(smichaud)
(Reporter)

Comment 21

3 years ago
I also tested on a Mac 10.9. This patch doesn't break Firefox on 10.9.
Hi Jonathan.  I'm currently on vacation.  I'll review and test your patch when I get back -- sometime next week.
(Reporter)

Comment 23

3 years ago
(In reply to Steven Michaud from comment #22)
> Hi Jonathan.  I'm currently on vacation.  I'll review and test your patch
> when I get back -- sometime next week.

No problem. Happy Holidays! :)
Comment on attachment 8541436 [details] [diff] [review]
Generate zh-TW.lproj instead of zh.lproj

This will need to be reviewed by a build peer so requesting review from Ted.

Ted, if there is someone else you think should review this please hand the review over to them.
Attachment #8541436 - Flags: review?(ted)
Comment on attachment 8541436 [details] [diff] [review]
Generate zh-TW.lproj instead of zh.lproj

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

I'm going to punt this to Pike, unless he's not interested.
Attachment #8541436 - Flags: review?(ted) → review?(l10n)

Comment 26

3 years ago
Comment on attachment 8541436 [details] [diff] [review]
Generate zh-TW.lproj instead of zh.lproj

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

I'd be surprised if this wouldn't affect our Spanish localizations, Bengali, Portuguese. Probably visually less obvious, but I'd expect the same outcome.

I could speculate on this better if someone could explain what's actually happening.

Technically, I wonder why the changes in l10n.mk would be necessary. There should be a central point where we hook this logic up.

I'm going to flag this with a tentative f-, not because I can actually point at a wrong thing, but because I'd rather be convinced in more detail that it's right. (And doesn't regress older versions of OSX)
Attachment #8541436 - Flags: review?(l10n) → feedback-
Status: REOPENED → ASSIGNED
Comment on attachment 8541436 [details] [diff] [review]
Generate zh-TW.lproj instead of zh.lproj

This is wrong as it stands -- the zh-TW package's lproj file should be
named "zh_TW.lproj", not "zh-TW.lproj".  zh_TW.lproj and zh_CN.lproj
is the naming scheme used by Safari and Chrome going back at least to
OS X 10.6 (the earliest version we still support).

But otherwise I think it's fine as it stands, and I don't understand
Axel's objections.

> I'd be surprised if this wouldn't affect our Spanish localizations,
> Bengali, Portuguese. Probably visually less obvious, but I'd expect
> the same outcome.

I'm by no means a build system expert, especially wrt to doing
localized builds.  But I *really* don't understand what you're saying
here.  How could Jonathan's patch possibly effect these localizations?

> Technically, I wonder why the changes in l10n.mk would be
> necessary.

As I said above, I know very little about how localized builds are
done.  But just looking at the code, it seems that en.lproj needs to
be temporarily renamed as per the destination locale for the
"update_packaging" command to work correctly.

Robert, can you shed any light on this?

> I could speculate on this better if someone could explain what's
> actually happening.

Here's my current understanding of this bug, and what we need to do to
fix it.

Safari and Chrome have long had all language support packaged into a
single distro.  So Safari and Chrome have both a zh_CN.lproj and a
zh_TW.lproj in their Contents/Resources/ directory.  But Firefox has
long had seperate distros for each localization, each of which
contains a single, appropriately named *.lproj file.  Appropriately
but not entirely correctly.  If a given localization has more than one
"variant" (e.g. zh_CN and zh_TW), *both* parts of its name should be
used.  Firefox, however, has long (and incorrectly) used just the
first part of the name (e.g. zh.lproj instead of zh_CN.lproj or
zh_TW.lproj).

I don't know whether this incorrect naming has caused trouble for
other locales.  But I do know (from this bug) that it seems *not* to
have caused trouble for the two Chinese locales (zh_CN and zh_TW)
until Yosemite was released.  As of the Yosemite release, it seems
that the OS interprets the filename zh.lproj as synonymous with
zh_CN.lproj (at least for some purposes -- for example the language
used in the system file picker).  To stop this happening, we need to
name the zh.lproj file for the zh_TW localization correctly -- we need
to name it zh_TW.lproj.
Attachment #8541436 - Flags: review?(smichaud) → review-
Flags: needinfo?(robert.strong.bugs)
Flags: needinfo?(l10n)
(In reply to Steven Michaud from comment #27)
> Comment on attachment 8541436 [details] [diff] [review]
> Generate zh-TW.lproj instead of zh.lproj
> 
> This is wrong as it stands -- the zh-TW package's lproj file should be
> named "zh_TW.lproj", not "zh-TW.lproj".  zh_TW.lproj and zh_CN.lproj
> is the naming scheme used by Safari and Chrome going back at least to
> OS X 10.6 (the earliest version we still support).
> 
> But otherwise I think it's fine as it stands, and I don't understand
> Axel's objections.
> 
> > I'd be surprised if this wouldn't affect our Spanish localizations,
> > Bengali, Portuguese. Probably visually less obvious, but I'd expect
> > the same outcome.
> 
> I'm by no means a build system expert, especially wrt to doing
> localized builds.  But I *really* don't understand what you're saying
> here.  How could Jonathan's patch possibly effect these localizations?
I think the concern is that there are other locales that could be affected by this (at least I am concerned) but I'll let Axel answer.

> > Technically, I wonder why the changes in l10n.mk would be
> > necessary.
> 
> As I said above, I know very little about how localized builds are
> done.  But just looking at the code, it seems that en.lproj needs to
> be temporarily renamed as per the destination locale for the
> "update_packaging" command to work correctly.
> 
> Robert, can you shed any light on this?
Not much since I didn't write it except to state the obvious that this is where the directory names for these files is decided upon, etc.
Flags: needinfo?(robert.strong.bugs)
(In reply to Steven Michaud from comment #27)
> > I'd be surprised if this wouldn't affect our Spanish localizations,
> > Bengali, Portuguese. Probably visually less obvious, but I'd expect
> > the same outcome.
> 
> I'm by no means a build system expert, especially wrt to doing
> localized builds.  But I *really* don't understand what you're saying
> here.  How could Jonathan's patch possibly effect these localizations?

Rephrasing Pike's comment: «I'd be surprise if *THIS BUG* wouldn't affect our Spanish localizations, etc.» (the bug, not the patch).

In other words: we're probably failing for all locales with language+country code (es-ES, es-AR, pt-PT, pt-BR, bn-IN, bn-BD, etc.). 
Maybe it's just less noticeable.
Flags: needinfo?(l10n)
(Reporter)

Comment 30

3 years ago
Created attachment 8547318 [details] [diff] [review]
Generate zh_TW.lproj instead of zh.lproj

I've changed zh-TW to zh_TW as Steve pointed out.

I can't verify if other languages are affected because I can't read them. You can call this patch a workaround for Taiwanese.

P.S. Taiwanese people really don't like reading zh_CN characters.
Attachment #8541436 - Attachment is obsolete: true
Attachment #8547318 - Flags: review?(smichaud)
Attachment #8547318 - Flags: review?(l10n)

Comment 31

3 years ago
Created attachment 8547332 [details]
pt-BR & pt-PT trial

Hi, I'd test both pt-BR/pt-PT and es-ES/es-MX locales, I'd confirm they're not affect by this bug. Please see the attach image, whether OS X's locale, pt-BR and pt-PT build of Firefox always use the correct "Save As" system dialog (can be distinguish by title) in their locales.

Comment 32

3 years ago
Created attachment 8547376 [details]
trial for es-ES/es-MX

Here is test result for es-ES/es-MX, according to translate in "Formato" in save dialog, es-* also not been affected by this bug.

Comment 33

3 years ago
Comment on attachment 8547376 [details]
trial for es-ES/es-MX

Please ignore this test case, just realized that string in "Formato" does not came from OS X system dialog.
Attachment #8547376 - Attachment is obsolete: true
Let's confine this bug to the zh_TW case.

There may very well be issues with pt-BR/pt-PT and es-ES/es-MX (and so forth).  But let's open another bug for dealing with them.  Unless we do so, we risk delaying a fix for the zh_TW case for a very long time.
Comment on attachment 8547318 [details] [diff] [review]
Generate zh_TW.lproj instead of zh.lproj

This, too, won't work as it stands.

Shortly I'll post what I had in mind ... once I can figure out how to test it.
Attachment #8547318 - Flags: review?(smichaud) → review-

Comment 36

3 years ago
Comment on attachment 8547318 [details] [diff] [review]
Generate zh_TW.lproj instead of zh.lproj

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

What smichaud said.

Also, it seems that OSX uses both - and _, but for different reasons, https://developer.apple.com/library/ios/documentation/MacOSX/Conceptual/BPInternational/LanguageandLocaleIDs/LanguageandLocaleIDs.html#//apple_ref/doc/uid/10000171i-CH15-SW1.

In our use-case, we shouldn't need '_' as a separator for region, as we're only interested in the language aspect.
Attachment #8547318 - Flags: review?(l10n)
Created attachment 8548474 [details] [diff] [review]
Fix

How about this, Axel?  It works in my tests (about which more in a
later comment).

> In our use-case, we shouldn't need '_' as a separator for region, as
> we're only interested in the language aspect.

Maybe *we* don't need it, but the *OS* does.  Furthermore, other apps
all use the underscore convention for *.lproj file names that specify
both language and region.  And, as I mentioned above in comment #27
(particularly the last two paragraphs), we need to use the full
zh_TW.lproj name to fix this bug.
Attachment #8547318 - Attachment is obsolete: true
Attachment #8548474 - Flags: review?(l10n)
Axel gave me the following doc to help me figure out how to test this bug's patch:
https://developer.mozilla.org/en-US/docs/Creating_a_Language_Pack

But apparently it (including its links) is not complete, because I couldn't get tests to work along those lines.  Instead I ended up doing the following:

1) Start with a source tree for something that has already been fully localized -- say the most recent release or a current beta tree.
2) Create a l10n-central directory in the top level of the source tree and cd to it.
3) hg clone http://hg.mozilla.org/l10n-central/zh-TW/
4) Add the following two lines to your mozconfig file:
   ac_add_options --with-l10n-base=../l10n-central
   ac_add_options --enable-ui-locale=zh-TW
5) Build the tree as you usually would.
6) Change to the resulting objdir and run "make package".
7) Notice a zh-TW dmg in objdir/dist

Comment 39

3 years ago
Comment on attachment 8548474 [details] [diff] [review]
Fix

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

I'm having a hard time to find real doc, but stumbling upon http://www.ibabbleon.com/iOS-Language-Codes-ISO-639.html for the iOS project makes me think we may not have to resort to _.

What may be the case is that zh-TW isn't good, but zh-Hant would be (and zh-CN -> zh-Hans).

I think we should make the switch between shortened language code and longer code based on shortened code.

I.e., the switch should be based upon $(AB) == 'zh'.

I also think we should use this logic for bn, en, pt, sr, zh, with zh potentially needing to map from region to script codes. 'sr' doesn't currently play, but might at some time in the future, once we support both Cyrl and Latn script for Serbian.

I'd love to know how test results go for zh, zh-TW, zh-Hant, zh_TW, zh-Hant_TW.

I'd look deeper, but I'm not on yosemite myself yet, and I'm not rushing to move over yet.

Also looping in gps, the build stuff is really his domain of expertise, and in particular in finding the right way to factor this into build and packaging he might have opinions.
Attachment #8548474 - Flags: review?(l10n) → feedback?(gps)
The underscore convention is used by *all* other apps on OS X for *.lproj file names.  That's reason enough for us to use it, too.
And what's the objection to using the underscore, anyway?
Actually I did manage to find one app that uses pr-BT.lproj, pt-PT.lproj, ru-RU.lproj and uk-UA.lproj -- gfxCardStatus (https://gfx.io/).  But even it also uses zh_TW.lproj and zh_CN.lproj.

I did find a few apps that use zh-Hans.lproj and zh-Hant.lproj.  Possibly if there's a valid objection to using zh_TW.proj and zh_CN.lproj, we could use these instead.  But we really do need to see a valid objection.

I'll come back shortly with results of testing zh-Hans.lproj and zh-Hant.lproj on Yosemite.
In my very brief testing, the name zh-Hant.proj seems to work as well on Yosemite as zh_TW.proj.  (You get the appropriate kind of Chinese characters 繁體字 in the file picker.)
proj -> lproj

Comment 45

3 years ago
Apple's doc's are a bit ambigous (as usual), but they do tell you here to use underscore:  https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFBundles/BundleTypes/BundleTypes.html#//apple_ref/doc/uid/10000123i-CH101-SW7

"Within the Resources directory of an OS X bundle (or the top-level directory of an iOS application bundle), you can create one or more language-specific project subdirectories to store language- and region-specific resources. The name of each directory is based on the language and region of the desired localization followed by the .lproj extension. To specify the language and region, you use the following format:

    language_region.lproj

The language portion of the directory name is a two-letter code that conforms to the ISO 639 conventions. The region portion is also a two-letter code but it conforms to the ISO 3166 conventions for designating specific regions"

Comment 46

3 years ago
After reading https://developer.apple.com/library/ios/documentation/MacOSX/Conceptual/BPInternational/LanguageandLocaleIDs/LanguageandLocaleIDs.html#//apple_ref/doc/uid/10000171i-CH15-SW1, my guess is that zh-Hant works because it's a valid script language ID.

[language designator]_[region designator] (zh_TW)
[language designator]-[script designator] (zh-Hant)
[language designator]-[script designator]_[region designator] (zh-Hant_TW)

Comment 47

3 years ago
Created attachment 8548547 [details]
Thunderbird may be affected by this bug

I wonder if Thunderbird is affected by this bug?
The attachment is the menu from Thunderbird. It shows the simplified Chinese.
If it is affected, we need to open a new one.

Comment 48

3 years ago
Created attachment 8548602 [details]
TB 31.4.0 page setup dialog

(In reply to Thomasy from comment #47)
> Created attachment 8548547 [details]
> Thunderbird may be affected by this bug
> 
> I wonder if Thunderbird is affected by this bug?
> The attachment is the menu from Thunderbird. It shows the simplified Chinese.
> If it is affected, we need to open a new one.

Confirmed that tb also been affected (and can be fixed by rename zh.lproj to zh-Hant.lproj). File a new bug please?
Comment on attachment 8548474 [details] [diff] [review]
Fix

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

I didn't go through the entire history of this bug. But if this hack is necessary, it's necessary.

glandium is doing l10n foo this quarter. You may want to run final review past him.
Attachment #8548474 - Flags: feedback?(gps) → feedback+
(Reporter)

Updated

3 years ago
Attachment #8548474 - Flags: review?(mh+mozilla)
Comment on attachment 8548474 [details] [diff] [review]
Fix

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

Hacks can be nice to get going, but it would be better to avoid confusing situations. The first thing that is confusing is that AB is being used, which implies it's the first part of AB_CD, which with this hack, it's not anymore. OTOH, looking at the codebase, AB is only used for those lproj directories for mac. So I'd rather completely rename AB to something else more relevant to what it is.

Now, is there actually a reason this should be limited to traditional chinese, besides history? Shouldn't we just rename those lproj directories for all locales? How does that work on older versions of OSX? Finally, can the situation wrt _ vs - be clarified?
Attachment #8548474 - Flags: review?(mh+mozilla) → review-
> Now, is there actually a reason this should be limited to traditional chinese, besides history?

I don't want for this bug to take forever to fix -- for the best to be enemy of the good.

Yes, someone, at some point, should investigate renaming *all* lproj directories with [language designator]_[region designator].  This won't be me, and in any case I expect it'll take somewhere between a long time and the heat death of the universe.

In the meantime we should land some kind of hack to fix this bug, which will just effect the zh-TW (aka zh_TW) distro.  I'll try to come up with a better name for AB and post my work here.
> can the situation wrt _ vs - be clarified?

Several interesting and helpful docs have been posted above, in comment #36, comment #45 and comment #46.  But ultimately we should use what works, and the best guide to that is what Apple's own apps do -- they use language_region.lproj (instead of just language.lproj) where there is any chance of ambiguity.
Created attachment 8552768 [details] [diff] [review]
Fix rev1

How's this?
Attachment #8548474 - Attachment is obsolete: true
Attachment #8552768 - Flags: review?(mh+mozilla)
Comment on attachment 8552768 [details] [diff] [review]
Fix rev1

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

::: browser/app/Makefile.in
@@ +85,5 @@
>  endif
>  
>  AB_CD = $(MOZ_UI_LOCALE)
>  
> +ifeq (zh-TW,$(AB_CD))

I still don't get why this needs to be specific to one locale. That is, I don't get why that would solve a problem for chinese and not work for other locales at the same time. Is OSX handling of locales with variable geometry?

@@ +88,5 @@
>  
> +ifeq (zh-TW,$(AB_CD))
> +LPROJ_ROOT := $(subst -,_,$(AB_CD))
> +else
> +LPROJ_ROOT := $(firstword $(subst -, ,$(AB_CD)))

you /could/ save some command line length by setting e.g. LPROJ := Contents/Resources/$(LPROJ_ROOT).lproj

@@ +89,5 @@
> +ifeq (zh-TW,$(AB_CD))
> +LPROJ_ROOT := $(subst -,_,$(AB_CD))
> +else
> +LPROJ_ROOT := $(firstword $(subst -, ,$(AB_CD)))
> +endif

b2g/app/Makefile.in needs the same code.

::: browser/installer/Makefile.in
@@ +132,5 @@
> +ifeq (zh-TW,$(AB_CD))
> +LPROJ_ROOT := $(subst -,_,$(AB_CD))
> +endif
> +endif
> +DEFINES += -DLPROJ_ROOT=$(LPROJ_ROOT)

b2g/installer/Makefile.in needs the same.

::: toolkit/locales/l10n.mk
@@ +117,5 @@
>  	$(STUB_HOOK)
>  endif
>  	$(PYTHON) $(MOZILLA_DIR)/toolkit/mozapps/installer/l10n-repack.py $(STAGEDIST) $(DIST)/xpi-stage/locale-$(AB_CD) \
>  		$(if $(filter omni,$(MOZ_PACKAGER_FORMAT)),$(if $(NON_OMNIJAR_FILES),--non-resource $(NON_OMNIJAR_FILES)))
> +ifneq (en,$(LPROJ_ROOT))

There's another bug that I think is touching around this that will conflict here.
Attachment #8552768 - Flags: review?(mh+mozilla) → feedback+
Created attachment 8555525 [details] [diff] [review]
Fix rev2

> you /could/ save some command line length by setting e.g. LPROJ :=
> Contents/Resources/$(LPROJ_ROOT).lproj

My new patch does this.

> b2g/app/Makefile.in needs the same code.

Done.

> b2g/installer/Makefile.in needs the same.

No.  Not as best I can tell.

> There's another bug that I think is touching around this that will
> conflict here.

I have no idea what this is, nor (with my limited knowledge of the
build system) will I be able to find out.  Let's just wait until this
issue comes up, and deal with it then.

> I still don't get why this needs to be specific to one locale. That
> is, I don't get why that would solve a problem for chinese and not
> work for other locales at the same time. Is OSX handling of locales
> with variable geometry?

I've already covered this several times.  But let me try again:

This particular bug happens because the OS changed its behavior in the
latest major version (OS X Yosemite).  We don't know why this
happened, and it will take much more than a reasonable amount of time
to find out.  In many respects (including this one), the OS behaves
like a black box.

But we've found that Apple apps (and many others) use the
language_region.lproj naming convention whenever there's any
possibility of ambiguity (unlike Mozilla apps, which have always used
the language.lproj naming convention).  We've also found that using
that convention ourselves fixes this particular bug.

Yes, it may be more correct to use the language_region.lproj naming
convention for *all* our localizations.  And doing so may fix other
bugs (including ones that haven't yet been reported).  But, since
we're dealing with a black box, we don't know what other, possibly
undesireable behavior changes may happen when we start using the
language_region.lproj naming convention.

We only have a very limited amount of time we can devote to fixing
this bug -- or at least I do.  We (or more correctly I) probably *do*
have time (and the requisite language knowledge) to deal with any
fallout this patch might cause on the zh-TW localization.  And in any
case we already know that using the language_region.lproj naming
convention in the zh-TW localization has substantial benefits (it
fixes this bug).  But I certainly don't have the time (and the
expertise) to investigate the implications of using the
language_region.lproj naming convention for all localizations, or to
deal with the possible fallout after such a patch lands.  I doubt
anyone else does, either (at least not now).

So if we refuse to hack the zh-TW localization to fix this bug, it's
likely to remain unfixed forever.
Attachment #8552768 - Attachment is obsolete: true
Comment on attachment 8555525 [details] [diff] [review]
Fix rev2

I don't know who to ask to review this patch.

Feel free to do it yourself, Mike.  But I noticed you only gave me a feedback+ on my previous patch.
Attachment #8555525 - Flags: review?(gps)
Assignee: jhao → smichaud
(In reply to Steven Michaud from comment #55)
> Created attachment 8555525 [details] [diff] [review]
> Fix rev2
> 
> > you /could/ save some command line length by setting e.g. LPROJ :=
> > Contents/Resources/$(LPROJ_ROOT).lproj
> 
> My new patch does this.
> 
> > b2g/app/Makefile.in needs the same code.
> 
> Done.
> 
> > b2g/installer/Makefile.in needs the same.
> 
> No.  Not as best I can tell.

There's probably a bug to file then. Either b2g/app is doing something useless, or the package manifest is missing the @AB@ entry.

> > There's another bug that I think is touching around this that will
> > conflict here.
> 
> I have no idea what this is, nor (with my limited knowledge of the
> build system) will I be able to find out.  Let's just wait until this
> issue comes up, and deal with it then.

Bug 1115892. Rebase your patch, you'll get a conflict.
 
> > I still don't get why this needs to be specific to one locale. That
> > is, I don't get why that would solve a problem for chinese and not
> > work for other locales at the same time. Is OSX handling of locales
> > with variable geometry?
> 
> I've already covered this several times.  But let me try again:
> 
> This particular bug happens because the OS changed its behavior in the
> latest major version (OS X Yosemite).  We don't know why this
> happened, and it will take much more than a reasonable amount of time
> to find out.  In many respects (including this one), the OS behaves
> like a black box.
> 
> But we've found that Apple apps (and many others) use the
> language_region.lproj naming convention whenever there's any
> possibility of ambiguity (unlike Mozilla apps, which have always used
> the language.lproj naming convention).  We've also found that using
> that convention ourselves fixes this particular bug.
> 
> Yes, it may be more correct to use the language_region.lproj naming
> convention for *all* our localizations.  And doing so may fix other
> bugs (including ones that haven't yet been reported).  But, since
> we're dealing with a black box, we don't know what other, possibly
> undesireable behavior changes may happen when we start using the
> language_region.lproj naming convention.
> 
> We only have a very limited amount of time we can devote to fixing
> this bug -- or at least I do.  We (or more correctly I) probably *do*
> have time (and the requisite language knowledge) to deal with any
> fallout this patch might cause on the zh-TW localization.  And in any
> case we already know that using the language_region.lproj naming
> convention in the zh-TW localization has substantial benefits (it
> fixes this bug).  But I certainly don't have the time (and the
> expertise) to investigate the implications of using the
> language_region.lproj naming convention for all localizations, or to
> deal with the possible fallout after such a patch lands.  I doubt
> anyone else does, either (at least not now).
> 
> So if we refuse to hack the zh-TW localization to fix this bug, it's
> likely to remain unfixed forever.

If you want to land something in beta for next release, sure, make it zh-TW specific. But I see no reason to land something zh-TW specific on central.
(In reply to Steven Michaud from comment #56)
> Comment on attachment 8555525 [details] [diff] [review]
> Fix rev2
> 
> I don't know who to ask to review this patch.
> 
> Feel free to do it yourself, Mike.  But I noticed you only gave me a
> feedback+ on my previous patch.

That was f+ as "close to r+, but I want to see next iteration".
Comment on attachment 8555525 [details] [diff] [review]
Fix rev2

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

Please rebase.
Attachment #8555525 - Flags: review?(gps)

Comment 60

3 years ago
Hi, do we have any progress on this bug? It's been almost 2 versions since the patch had been submitted, how can we landing it?
I've simply run out of time to spend on this bug -- for the time being it's been superceeded by other, more urgent bugs.  I thought (and still think) that it should be relatively simple to fix, which is why I started working on it when I did.  But not everyone seems to agree.

I expect it'll be at least a month before I once again have time to work on this bug.  If someone else wants to pick up where I left off, please feel free to do so.
Assignee: smichaud → nobody

Comment 62

3 years ago
Hi Mike, can we land the simple zh-TW specific patch to catch next train? 

The problem had been exist for 3 versions and it's really annoying and rude for all zh-TW users.
Othree will try to rebase the patch in comment 59 and I will help him to push this to try to get a verification.
Assignee: nobody → othree
(Assignee)

Comment 64

2 years ago
Created attachment 8633452 [details]
photo_2015-07-14_20-43-00.jpg

Current patch result
(Assignee)

Comment 65

2 years ago
Try rebase the patch and build with zh-TW successful.

Result screen shot: https://bug1089363.bugzilla.mozilla.org/attachment.cgi?id=8633452

Have some issue on create the patch file.
Will upload it later when ready.
(Assignee)

Comment 66

2 years ago
Created attachment 8633855 [details] [diff] [review]
tw-img-dl-dialog.patch

Patch file base on latest b2g/app/Makefile.in
Comment on attachment 8633855 [details] [diff] [review]
tw-img-dl-dialog.patch

Are you sure this patch includes all the changes needed? Looks like just a subset of attachment 8555525 [details] [diff] [review].

If so I would push this to Try.
Flags: needinfo?(othree)
(Assignee)

Comment 68

2 years ago
Yes, the Makefile.in is keep reducing size.

There are only 79 lines now.
The old one have more than 120 lines. (see attach: Fix rev2)
Flags: needinfo?(othree)
The patch is incomplete. Fix rev2 is complete, but outdated. Please rebase Fix rev2.
(Assignee)

Comment 70

2 years ago
I saw the other files. Those files are not conflict with current reversion.
Trying merge it.
Talk to Mike over irc on #developers, here are the next steps:

1. Create a complete patch of the rebase of Fix rev 2
2. Verify locally and upload PNGs so people can verify the build is built with zh-TW and the text in dialog are fixed. Maybe include the About Firefox dialog in screenshot?
3. Try push is not needed because Try does not test l10n builds for us.
(Assignee)

Comment 72

2 years ago
Created attachment 8633931 [details]
tw-img-dl-dialog.png

Fix rev3 result screenshot
(Assignee)

Comment 73

2 years ago
Created attachment 8633932 [details] [diff] [review]
Fix rev3.patch

Fix rev 3, 4 files effected 
Rev 2 effected 5 files, rev 3 not change browser/installer/package-manifest.in
(Assignee)

Comment 74

2 years ago
Comment on attachment 8633932 [details] [diff] [review]
Fix rev3.patch

Current makefile are all using `AB` as the locale variable.
So browser/installer/package-manifest.in is not changed.
Attachment #8633932 - Flags: review?(mh+mozilla)
Thanks for the hard work!
Comment on attachment 8633932 [details] [diff] [review]
Fix rev3.patch

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

See the first sentences of comment 50, which explains why the package-manifest.in change, and the corresponding Makefile.in change were done.
Attachment #8633932 - Flags: review?(mh+mozilla)
(Assignee)

Comment 77

2 years ago
Created attachment 8633959 [details] [diff] [review]
Fix rev4.patch

Use ${LPROJ_ROOT} instead of ${AB}.
Effected 5 files.
Attachment #8633855 - Attachment is obsolete: true
Attachment #8633932 - Attachment is obsolete: true
Attachment #8633959 - Flags: review?(mh+mozilla)
Comment on attachment 8633959 [details] [diff] [review]
Fix rev4.patch

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

Please attribute to smichaud when landing.
Attachment #8633959 - Flags: review?(mh+mozilla) → review+
Also please file a followup to do the same on for other locales.
(Assignee)

Comment 80

2 years ago
Created attachment 8635236 [details] [diff] [review]
Fix rev5.patch, change attribute user
(Assignee)

Comment 81

2 years ago
Created attachment 8635242 [details] [diff] [review]
Fix rev5.patch, change attribute user, add reviewer.
Attachment #8635236 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8635242 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Attachment #8633959 - Attachment is obsolete: true
Attachment #8555525 - Attachment is obsolete: true

Comment 82

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3daf8050780
Keywords: checkin-needed
Backed out for breaking OSX B2G desktop builds.
https://treeherder.mozilla.org/logviewer.html#?job_id=11842723&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/154a12a7a91c
(Assignee)

Comment 84

2 years ago
Created attachment 8635719 [details] [diff] [review]
Fix rev6.patch, fix b2g build
Attachment #8635242 - Attachment is obsolete: true
Attachment #8635719 - Flags: review?(mh+mozilla)
(Assignee)

Comment 85

2 years ago
Test build b2g and nightly using en and zh_TW are all ok in my local machine.

Updated

2 years ago
Blocks: 1185268

Updated

2 years ago
Blocks: 1185270
Comment on attachment 8635719 [details] [diff] [review]
Fix rev6.patch, fix b2g build

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

::: b2g/app/Makefile.in
@@ +28,5 @@
>  endif
>  
>  AB_CD = $(MOZ_UI_LOCALE)
>  
>  AB := $(firstword $(subst -, ,$(AB_CD)))

You don't need to keep this.

@@ +41,5 @@
>  	rm -rf $(DIST)/$(APP_NAME).app
>  
>  libs-preqs = \
>    $(call mkdir_deps,$(DIST)/$(APP_NAME).app/Contents/MacOS) \
> +	$(call mkdir_deps,$(DIST)/$(APP_NAME).app/$(LPROJ)) \

Please keep the 2-space indentation.
Attachment #8635719 - Flags: review?(mh+mozilla)
(Assignee)

Comment 87

2 years ago
Created attachment 8636977 [details] [diff] [review]
Fix rev7.patch
Attachment #8635719 - Attachment is obsolete: true
Attachment #8636977 - Flags: review?(mh+mozilla)
Attachment #8636977 - Flags: review?(mh+mozilla) → review+
(In reply to othree (MozTW) from comment #87)
> Created attachment 8636977 [details] [diff] [review]
> Fix rev7.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=101be291a1bb

Set try to build EVERYTHING w/o running tests.
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 89

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1587270e7fc
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a1587270e7fc
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago2 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
\o/ Thanks :othree, :smichaud, and :glandium!

Comment 92

2 years ago
Please consider uplift to branches including ESR38.
Flags: needinfo?(smichaud)
Flags: needinfo?(othree)
Philip, neither othree nor I can make that decision.  Why don't you request uplift to ESR38?
Flags: needinfo?(smichaud)
Flags: needinfo?(othree)

Updated

2 years ago
Blocks: 1192629

Comment 94

2 years ago
After greping the keyword ("AB" and "firstword" and without "LPROJ_ROOT") and "@AB@" the bug effects the following parts, which will be solved in the separate bugs.
#grep AB ./* -R | grep firstword | grep -v LPROJ_ROOT

./im/app/Makefile.in:AB := $(firstword $(subst -, ,$(AB_CD)))
./mail/app/Makefile.in:AB := $(firstword $(subst -, ,$(AB_CD)))
./mail/installer/Makefile.in:AB = $(firstword $(subst -, ,$(AB_CD)))
./suite/app/Makefile.in:AB := $(firstword $(subst -, ,$(AB_CD)))
./suite/installer/Makefile.in:AB = $(firstword $(subst -, ,$(AB_CD)))

#grep @AB@ ./* -R -I

./mail/installer/package-manifest.in:@RESPATH@/@AB@.lproj/*
./suite/installer/package-manifest.in:@RESPATH@/@AB@.lproj/*
You need to log in before you can comment on or make changes to this bug.