Closed Bug 451426 Opened 16 years ago Closed 16 years ago

Incorrect rendering of some Japanese characters when using downloadable fonts (@font-face)

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1b3

People

(Reporter: a-yoshi, Assigned: jfkthame)

References

Details

(Keywords: fixed1.9.1)

Attachments

(3 files, 2 obsolete files)

User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080818032051 Minefield/3.1a2pre Build Identifier: Some Japanese characters were rendered incorrectly when using downloadble fonts. This is how downloadable fonts work. At first, when the fonts are not downloaded, text will be rendered using default font. But once the download finishes, they will be rendered again using the downloaded font. The problem is that on this second rendering, some Japanese characters are rendered incorrectly, when downloaded fonts does not contain Japanese characters within it. Japanese characters should be rendered using default Japanese fonts (since the downloaded font does not contain Japanese characters), but it seems that they are using Chinese fonts instead of using Japanese fonts. It would be easier to tell the difference if you compare this text with the same text without applying the downloadable fonts. This only happens when you applied the patch v.0.8b for bug 441473 to the build tree. Reproducible: Always Steps to Reproduce: 1. Apply patch v.0.8b from bug 441473 and build 2. Make an HTML file in Japanese encoding (Shift_JIS, EUC-jp) 3. Write a text that contains Japanese characters in between Chinese characters and apply downloadable font to the text (make sure the font you download does not contain Japanese characters) 4. Open the HTML file from browser, and when the download is done, Japanese characters would be rendered incorrectly Actual Results: Japanese characters are rendered incorrectly, probably using Chinese fonts. Expected Results: Japanese characters should be rendered using Japanese fonts.
Depends on: 441473
Flags: wanted1.9.1?
Blocks: 441473
No longer depends on: 441473
I'm not sure whether we generally want bugs against stuff you get when applying patches in other bugs, but for something this big it may be useful.
Assignee: nobody → jdaggett
(In reply to comment #1) > I'm not sure whether we generally want bugs against stuff you get when applying > patches in other bugs, but for something this big it may be useful. > Akira and I hemmed and hawed over just that issue until J. Daggett asked us to write it up: https://bugzilla.mozilla.org/show_bug.cgi?id=441473#c46
Yeah, normally I wouldn't ask that these be separated but the patch is fairly large and I didn't want these issues to get lost in the noise.
Akira, could you attach a testcase for this?
Last 8 characters of each sentence are Chinese characters. Among them, 3rd, 7th and 8th character are rendered thinner compared to other characters. It would be more obvious if you can compare the same sentence using .test { font-family: "Comic Sans MS"; } instead of using downloadable fonts. Also, once you loaded this page, please focus on some other window. If you focus again of firefox window after about 1 minute, Japanese characters (フォントの拡張子って) in the middle of the sentence will suddenly be rendered in a different way. This only happens to the first sentence which is using downloadable font.
Flags: wanted1.9.1? → blocking1.9.1+
Priority: -- → P3
Attachment #335726 - Attachment mime type: text/html → text/html; charset=Shift_JIS
Akira, not sure I see the problem here. If this problem still occurs, could you post a screenshot, along with the exact set of steps you went through to get to that state? I see one line rendered in Comic Sans, one line rendered in the default font. In both cases fallback occurs for Japanese, Chinese and Korean characters. Given the defined styles, this appears correct.
(In reply to comment #6) I cannot reproduce the bug right now. That is because I've finished my internship and I don't have a Mac OS build now (I forgot to mention, but this problem only occurs on Mac OS). I'm sorry that I cannot attach the screenshot. If you could compare the two following test pages using reftest, the bug should reproduce. I'm sure it reproduced on patch v.0.9h. I hope you can see the difference. Test case for @font-face rules v.0.6 (bug 452780) https://bugzilla.mozilla.org/attachment.cgi?id=338203 In this attached test suite, you can find the following test cases. downloadable-font-japanese.html - using downloadable font (since it's using url:local(), it uses the system font as a consequence) downloadable-font-japanese-ref.html - using system font (ComicSansMS) > I see one line rendered in Comic Sans, one line rendered in the default font. > In both cases fallback occurs for Japanese, Chinese and Korean characters. > Given the defined styles, this appears correct. Yes, the fallback should occur. The point is at the last 8 characters (ABCDEFGH) in the sentence using Comic Sans. The 3rd, 7th and 8th characters should be rendered using Japanese fonts, since the encoding is specified as Shift_JIS. But when you use the @font-face rules, they are rendered using Chinese fonts. When you use the normal .test { font-family: "Comic Sans MS"; } rules, like in downloadable-font-japanese-ref.html, these characters are rendered using Japanese fonts.
No longer blocks: 441473
Screenshot showing the incorrect rendering of Japanese characters, created with current Minefield build on Mac OS X (10.5.5). The first line uses @font-face to access Bitstream Vera Sans; the 3rd, 7th and 8th Japanese characters are rendered from a different fallback font than the rest. The second line of the sample specifies Bitstream Vera Sans directly using font-family: "Bitstream Vera Sans", and does not show the problem. (The font difference is most obvious in the final character.) I observed that if I use Zoom In or Zoom Out to change the size of the displayed text, both lines render correctly; subsequently using Reload to refresh the page brings back the problem with those three characters on line 1. The exact set of characters that are rendered incorrectly is variable; on some occasions, especially when switching back into Minefield after activating another application for some time, I have seen a different fallback font used for the first Japanese character as well. This behavior seems a bit unpredictable, however. Safari on the same machine consistently renders both lines correctly (matching line 2 of the Minefield display).
Due to an uninitialized flag, there is a strong probability that in the case where a font is found in the mUserFontSet, gfxFontGroup::ForEachFontInternal will exit early while resolving font families (at line 1033 of gfxFont.cpp). In the test case given, this was leading to a truncated FontList containing only the downloaded font, and not the language-appropriate font for the generic sans-serif family; subsequently, gfxFontGroup::FindFontForChar would fall back to WhichPrefFontSupportsChar() rather than finding the appropriate Japanese font from the active font group. With this patch, which ensures that aborted is set to PR_FALSE until specifically used, the test document renders correctly and consistently. (An uninitialized local variable being the cause is also consistent with the observation that the behavior was not 100% predictable, and could vary with changing focus and redrawing of the frame even within a single run of the program.) NB: while investigating this, I was also puzzled by lines 1124-1126 of gfxFont.cpp: // if match, return if (selectedFont) return selectedFont.forget(); I don't see how selectedFont can have any useful value at this point in the function. Might this be an obsolete fragment of code that ought to be removed?
Attachment #346261 - Flags: superreview?
Attachment #346261 - Flags: review?
Attachment #346261 - Flags: superreview?(roc)
Attachment #346261 - Flags: superreview?
Attachment #346261 - Flags: review?(roc)
Attachment #346261 - Flags: review?
Patch looks good. > NB: while investigating this, I was also puzzled by lines 1124-1126 of > gfxFont.cpp: > > // if match, return > if (selectedFont) > return selectedFont.forget(); > > I don't see how selectedFont can have any useful value at this point in the > function. Might this be an obsolete fragment of code that ought to be removed? This looks obsolete, feel free to trim it out. One small nit, could you modify your .hgrc file to include the lines below, as noted in the Mercurial FAQ https://developer.mozilla.org/en/Mercurial_FAQ [diff] git = 1 [defaults] diff=-p -U 8 This will give your diff's more context.
Attachment #346261 - Flags: superreview?(roc)
Attachment #346261 - Flags: superreview+
Attachment #346261 - Flags: review?(roc)
Attachment #346261 - Flags: review+
Comment on attachment 346261 [details] [diff] [review] fix for uninitialized variable that caused premature return when resolving fonts great!
Revised patch using preferred diff settings. Also eliminated the obsolete code mentioned in comment #10, although I believe that code was harmless in practice because of nsRefPtr's initialization to null.
Attachment #346261 - Attachment is obsolete: true
Assignee: jdaggett → jfkthame
See bug 452870#c17 for reftest related to this bug.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: [needs landing]
Whiteboard: [needs landing]
Version: unspecified → Trunk
Attachment #346320 - Attachment is obsolete: true
Landed on mozilla-central (and thus also on mozilla-1.9.1): http://hg.mozilla.org/mozilla-central/rev/77825d347650 It turns out that this made a bunch of reftests unexpectedly pass, showing that this fixed bug 465409: http://hg.mozilla.org/mozilla-central/rev/a036f2285660 I still need to investigate whether it also fixed bug 465408.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla1.9.1b3
It also fixed bug 465408 and the test that was random on x86-Mac, so I adjusted the reftest.list appropriately; there are now no longer any random tests in reftests/font-face/reftest.list : http://hg.mozilla.org/mozilla-central/rev/e29be5f4f686
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: