Closed
Bug 1104589
Opened 9 years ago
Closed 9 years ago
pinyin text tagged with zh-Latn-pinyin displays with Chinese bitmap font
Categories
(Core :: Graphics: Text, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: jtd, Assigned: jfkthame)
References
()
Details
Attachments
(3 files, 1 obsolete file)
564 bytes,
text/html
|
Details | |
743 bytes,
patch
|
smontagu
:
review-
|
Details | Diff | Splinter Review |
2.63 KB,
patch
|
smontagu
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•9 years ago
|
||
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!
Reporter | ||
Comment 2•9 years ago
|
||
Reporter | ||
Comment 3•9 years ago
|
||
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).)
Reporter | ||
Comment 5•9 years ago
|
||
(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.
Assignee | ||
Comment 6•9 years ago
|
||
(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.
Reporter | ||
Comment 7•9 years ago
|
||
windows try build of the patch: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jdaggett@mozilla.com-644b4feb8dd7/try-win32/firefox-36.0a1.en-US.win32.installer.exe
Comment 8•9 years ago
|
||
(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 9•9 years ago
|
||
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-
Assignee | ||
Comment 10•9 years ago
|
||
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.
Assignee | ||
Comment 11•9 years ago
|
||
Something like this ought to do it, I think.
Attachment #8528313 -
Flags: review?(smontagu)
Comment 12•9 years ago
|
||
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)) {}"?
Assignee | ||
Comment 13•9 years ago
|
||
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)
Assignee | ||
Updated•9 years ago
|
Attachment #8528313 -
Attachment is obsolete: true
Attachment #8528313 -
Flags: review?(smontagu)
Updated•9 years ago
|
Attachment #8528324 -
Flags: review?(smontagu) → review+
Reporter | ||
Comment 14•9 years ago
|
||
Jonathan, could you push a try build? I'd like to have the original reporter test this.
Assignee | ||
Comment 15•9 years ago
|
||
I pushed https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7e3b9616fcd5 as I wanted to make sure existing reftests weren't affected.
Comment 16•9 years ago
|
||
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.
Assignee | ||
Comment 17•9 years ago
|
||
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)
Comment 18•9 years ago
|
||
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.
Assignee | ||
Comment 19•9 years ago
|
||
(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.)
Comment 20•9 years ago
|
||
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?
Comment 21•9 years ago
|
||
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.
Assignee | ||
Comment 22•9 years ago
|
||
(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.)
Comment 23•9 years ago
|
||
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!
Assignee | ||
Comment 24•9 years ago
|
||
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
Comment 25•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/59961a96fcb9
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.
Description
•