Closed Bug 1104589 Opened 5 years ago Closed 5 years ago

pinyin text tagged with zh-Latn-pinyin displays with Chinese bitmap font

Categories

(Core :: Graphics: Text, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jtd, Assigned: jfkthame)

References

()

Details

Attachments

(3 files, 1 obsolete file)

The wikipedia page included here displays with a very illegible Chinese bitmap font for the pinyin text (e.g. "Mandarin pinyin: Hǔzhuǎpài"). I suspect this is the result of the lang tag "zh-Latn-pinyin" mapping to a Chinese font and hence displaying with an embedded bitmap font (ew!).

<span xml:lang="zh-Latn-pinyin" lang="zh-Latn-pinyin">Hǔzhuǎpài</span>

Need to track down what lang group ends up in gfx, as the fix here might be to map zh-Latn to something different in the lang group handling code.
Generic font lookup occurs here:

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxTextRun.cpp#1581

This calls through to GetLanguageGroup to get the lang group string. The code in GetLanguageGroup only looks at lang tag and ignores subtags:

http://mxr.mozilla.org/mozilla-central/source/intl/locale/nsLanguageAtomService.cpp#114

The result is that it basically maps xx-yyy to xx!
Add an entry in the lang group resource to map zh-latn-pinyin to x-western
Attachment #8528207 - Flags: review?(smontagu)
(A somewhat broader comment -- but the whole langGroup concept feels like it's meant to represent something awfully similar to the ISO 15924 script codes -- and perhaps could be replaced by use of script codes (with some normalization).)
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #4)
> (A somewhat broader comment -- but the whole langGroup concept feels like
> it's meant to represent something awfully similar to the ISO 15924 script
> codes -- and perhaps could be replaced by use of script codes (with some
> normalization).)

To some degree, yes. But these lang groups are tied to the set of choices presented in the fonts dialog (Preferences > Content > Advanced...) *and* tied to font fallback lists (font.name.* and font.name-list.*). So it's not just replacing one set of codes with another, the pref UI for fonts (along with how to specify font fallback lists better) needs to be rethought/restructed in addition.
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #4)
> (A somewhat broader comment -- but the whole langGroup concept feels like
> it's meant to represent something awfully similar to the ISO 15924 script
> codes -- and perhaps could be replaced by use of script codes (with some
> normalization).)

Yes! See bug 556237.
(In reply to Jonathan Kew (:jfkthame) from comment #6)
> (In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from
> comment #4)
> > (A somewhat broader comment -- but the whole langGroup concept feels like
> > it's meant to represent something awfully similar to the ISO 15924 script
> > codes -- and perhaps could be replaced by use of script codes (with some
> > normalization).)
> 
> Yes! See bug 556237.

Yes!! It's really way beyond time we fixed that. I wonder if it would make a good GSOC project?
Comment on attachment 8528207 [details] [diff] [review]
patch, map pinyin to x-western

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

So even if we aren't going to solve bug 556237 completely in the near future, I think we should at least fix the code in GetLanguageGroup so that it matches a maximal substring, i.e. we can just add zh-latn=x-western in langGroups.properties (and ja-latn=x-western too while we're there) and not have to file new bugs next week for zh-Latn-wadegile or ja-Latn-hepburn.
Attachment #8528207 - Flags: review?(smontagu) → review-
That sounds analogous to what I did for finding hyphenation patterns, as we didn't have a proper BCP47-matching service to use. Basically, strip trailing subtags until we find a match.
Something like this ought to do it, I think.
Attachment #8528313 - Flags: review?(smontagu)
Comment on attachment 8528313 [details] [diff] [review]
Improve the mapping of lang tags to langGroup codes.

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

Yes, that's exactly what I had in mind

::: intl/locale/nsLanguageAtomService.cpp
@@ +125,2 @@
>          }
> +      } while (NS_FAILED(res));

I don't like the "if foo do {} while foo" construct much though. Can you recast this to just use "while (NS_FAILED(res)) {}"?
I originally wrote it that way, then changed it to avoid creating the |truncated| string before deciding whether we need to enter the loop. But actually, we don't need that string at all: we can just operate on |langStr| directly, as it's not used for anything further. So here's a simplified version.
Attachment #8528324 - Flags: review?(smontagu)
Attachment #8528313 - Attachment is obsolete: true
Attachment #8528313 - Flags: review?(smontagu)
Attachment #8528324 - Flags: review?(smontagu) → review+
Jonathan, could you push a try build? I'd like to have the original reporter test this.
I pushed https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7e3b9616fcd5 as I wanted to make sure existing reftests weren't affected.
I'm the original report. Sorry, no change in the new build: 

In the primary test Windows 7 configuration, the bitmapped glyphs are almost unreadable at 100% zoom. Zoomed larger (or at 100% and larger in Windows 8 or Ubuntu) all glyphs are smoother, but letters with diacritics are not on the baseline and in general do not form correctly.
That's odd, it seems to work as expected for me.

What build exactly were you testing? (Go to "about:buildconfig" and copy the "Built from ..." identifier.)

On your Windows 7 system, what is the default sans-serif font for Latin script? (See under Tools > Options > Content > Fonts & Colors: Advanced... > Fonts for: Latin)
This was the nightly test build https://hg.mozilla.org/releases/mozilla-release/rev/a4d51da32fe7

Also tested in public releases 33.1, 33.1.1 and 35.0a2. 

Default san-serif font is the standard default in all configurations: Arial, with pages allowed to choose their own fonts, and fallback encoding defaulting to current locale. Changing to Arial Unicode MS does not fix the problem, because in this case Firefox is getting embedded bitmap glyphs from somewhere else. 

Note that in Firefix 35 Dev edition on the Windows 7 system, and in all editions on other systems tested (Windows 8 and Ubuntu 14.04), the glyphs are not as illegible at 100% zoom, but the issues of baseline alignment and poor forming remain. 

If you zoom larger and see the baseline and forming issues, then this may indeed be working as expected due to the reliance on embedded bitmap glyphs, but it makes Firefox compare poorly to Chrome.
(In reply to Joe Katz from comment #18)
> This was the nightly test build
> https://hg.mozilla.org/releases/mozilla-release/rev/a4d51da32fe7

Ah -- that won't help with testing the proposed fix here: the patch hasn't landed in Nightly builds at this point. You'd need to test the tryserver build from http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-7e3b9616fcd5/try-win32/.

(You can either get the firefox-36.0a1.en-US.win32.installer.exe installer, which will install just like a normal Nightly build, or download the zip archive firefox-36.0a1.en-US.win32.zip, extract all the files, and then run firefox.exe from the resulting folder without actually installing it.)
Thanks. I think that's the build I received from John. The nightly builds may not identify themselves correctly.) Anyway, I tested one you pointed me to and got the same result. On your machine, even if you don't see dotty glyphs for whatever reason, when you zoom in do you see the baseline and general forming issues?
A further thought: replicating the worst-case bitmap on your own machine may be dependent on your screen resolution. 

To test this, I opened Firefox in Windows 8 within VirtualBox on my Windows 7 machine. The screen resolution is 1900 x 1200. The worst dotty glyphs are there in both operating systems at this resolution. On a Win8/Ubuntu machine with a 1980 x 1080 screen, it is not dotty just hard to read because it looks like two fonts mixed together as previously described. 

So this will always happen to some portion of users, and if they think of zooming in then it gets better but in IMHO ugly and for some users probably still hard to read.
(In reply to Joe Katz from comment #20)
> Thanks. I think that's the build I received from John. The nightly builds
> may not identify themselves correctly.) Anyway, I tested one you pointed me
> to and got the same result. On your machine, even if you don't see dotty
> glyphs for whatever reason, when you zoom in do you see the baseline and
> general forming issues?

Yes, if I use a current Nightly build; but no, if I use the test build from http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-7e3b9616fcd5/try-win32/. With that, the pinyin text is rendered in Arial and looks fine.

Please double-check that about:buildconfig shows that the version you're testing is "Built from https://hg.mozilla.org/try/rev/7e3b9616fcd5". If not, it's not the right build.

(Note that I think you need to completely quit any Firefox or Nightly version you're currently running before trying to launch a new one; otherwise it will not really run the new build but just "hand off" to a new window in the already-running program.)
Eureka!!! It is fixed in that build. Sorry for not quitting all other instances of Firefox before. If this patch is going to be in the next public release, then I for one say we can close this bug. Thank you all for your support!
Great -- glad to know it works for you. Sorry about the confusion with testing... it suddenly occurred to me that you might be caught out by that behavior. It's easy to be tricked, so that you're not testing what you think you are!

The fix here is currently on track to appear in Firefox 36.

https://hg.mozilla.org/integration/mozilla-inbound/rev/59961a96fcb9
Assignee: nobody → jfkthame
Target Milestone: --- → mozilla36
https://hg.mozilla.org/mozilla-central/rev/59961a96fcb9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.