550772 - fallback pref font handling for CJK fonts is broken

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Youhei Tooyama]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; ja-JP-mac; rv:1.9.1.8) Gecko/20100205 (Not Firefox/3.5) SeaMonkey/2.0.3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a3pre) Gecko/20100307 Minefield/3.7a3pre

The system font emulation has been broken on Mac.
See attachment file.

Reproducible: Always

Steps to Reproduce:
1.Update Trunk
fin
Actual Results:  
Broken the fonts

Expected Results:  
Thing correctly displayed

about:buildconfig

Source

Built from http://hg.mozilla.org/mozilla-central/rev/7a2802932585
Build platform
target
powerpc-apple-darwin9.2.0

Build tools
Compiler 	Version 	Compiler flags
gcc-4.2 -arch ppc 	gcc version 4.2.1 (Apple Inc. build 5564) 	-Wall -W -Wno-unused -Wpointer-arith -Wcast-align -W -Wno-long-long -gdwarf-2 -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fpascal-strings -fno-common -pthread -DNO_X11 -DNDEBUG -DTRIMMED -O3
g++-4.2 -arch ppc 	gcc version 4.2.1 (Apple Inc. build 5564) 	-fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -Wno-long-long -gdwarf-2 -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fpascal-strings -fno-common -fshort-wchar -pthread -DNO_X11 -DNDEBUG -DTRIMMED -O3

Configure arguments
--target=powerpc-apple-darwin9.2.0 --with-macos-sdk=/Developer/SDKs/MacOSX10.5.sdk --enable-application=browser --enable-update-channel=nightly --enable-update-packaging --enable-tests --enable-codesighs --disable-install-strip
Source

Built from http://hg.mozilla.org/mozilla-central/rev/7a2802932585
Build platform
target
i386-apple-darwin9.2.0

Build tools
Compiler 	Version 	Compiler flags
gcc-4.2 -arch i386 	gcc version 4.2.1 (Apple Inc. build 5564) 	-Wall -W -Wno-unused -Wpointer-arith -Wcast-align -W -Wno-long-long -gdwarf-2 -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fpascal-strings -fno-common -pthread -DNO_X11 -DNDEBUG -DTRIMMED -O3
g++-4.2 -arch i386 	gcc version 4.2.1 (Apple Inc. build 5564) 	-fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -Wno-long-long -gdwarf-2 -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fpascal-strings -fno-common -fshort-wchar -pthread -DNO_X11 -DNDEBUG -DTRIMMED -O3

Configure arguments
--target=i386-apple-darwin9.2.0 --with-macos-sdk=/Developer/SDKs/MacOSX10.5.sdk --enable-application=browser --enable-update-channel=nightly --enable-update-packaging --enable-tests --enable-codesighs --disable-install-strip

Comment 1

[Youhei Tooyama]

Attachment: Font Setting Window

Font Setting Window-1

Comment 2

[Youhei Tooyama]

Attachment: Most Visited Folder

Most Visited Folder

Comment 3

[Youhei Tooyama]

Attachment: Library Window

Library Window

Comment 4

[John Daggett (:jtd)]

Please add a clearer description of what you think the bug is here along with steps to reproduce the problem.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

regression range:
c9ff351b0738 (OK) - 1936b0d5f431 (NG)

(In reply to comment #4)
> Please add a clearer description of what you think the bug is here along with
> steps to reproduce the problem.

When you type any Japanese characters in searchbar, you can see Chinese font's glyphs, not Japanese font's.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

130a52770a7b is OK.
7170fe459a6c isn't OK.

http://hg.mozilla.org/mozilla-central/rev/7170fe459a6c
http://hg.mozilla.org/mozilla-central/rev/48d939fb59c3
http://hg.mozilla.org/mozilla-central/rev/580c63190d26
http://hg.mozilla.org/mozilla-central/rev/9ee18dec071b
http://hg.mozilla.org/mozilla-central/rev/cf1240a63d4e
http://hg.mozilla.org/mozilla-central/rev/c1cc02524fbb
http://hg.mozilla.org/mozilla-central/rev/92662f367b6e

Comment 7

[Boris Zbarsky [:bzbarsky]]

Bug 441110 seems most likely out of those....

Comment 8

[John Daggett (:jtd)]

(In reply to comment #7)
> Bug 441110 seems most likely out of those....

Confirmed that the changeset associated with this bug causes the problem:

http://hg.mozilla.org/mozilla-central/rev/580c63190d26

Comment 9

[John Daggett (:jtd)]

Attachment: testcase

Simple testcase.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/nsUnicodeRange.h

> 42 // The following constants define unicode subranges
> 43 // values below kRangeNum must be continuous so that we can map to 
> 44 // lang group directly.
> 45 // all ranges we care about should be defined under 32, that allows 
> 46 // us to store range using bits of a PRUint32

>     9.15 -const PRUint8   kRangeSetStart  =  30;    // range set definition starts from here
>     9.16 -const PRUint8   kRangeSetLatin  =  30;
>     9.17 -const PRUint8   kRangeSetCJK    =  31;
>     9.18 -const PRUint8   kRangeSetEnd    =  31;   // range set definition ends here
>     9.19 +const PRUint8   kRangeSetStart  =  31;    // range set definition starts from here
>     9.20 +const PRUint8   kRangeSetLatin  =  31;
>     9.21 +const PRUint8   kRangeSetCJK    =  32;
>     9.22 +const PRUint8   kRangeSetEnd    =  32;   // range set definition ends here

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

taking.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v1.0

The actual cause is the gfxPlatForm part. The enum shouldn't be used as bit mask right now.

I think that the nsUnicodeRange.h should be restored. I never found any problems, but that needs to change the rules, so, it's risky.

Comment 13

[John Daggett (:jtd)]

Attachment: patch, v.0.1, eliminate 32-bit dependency

Allow pref lang values outside [1..31].

Comment 14

[John Daggett (:jtd)]

Comment on attachment 431042 [details] [diff] [review]
Patch v1.0

Looks good.  Use "grow larger" rather than "grow up".

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v1.0.1

Thank you, John-san. I'll land the patch tonight.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://hg.mozilla.org/mozilla-central/rev/3e35314b310f

Comment 17

[Masatoshi Kimura [:emk]]

Sorry, I couldn't catch the error before landing 441110.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

If I reviewed that, I took same mistake. I think the actual cause of the regression is that the note isn't in the diff file. When we define some constants, we should write the note near an important constant rather than the top of the definitions.

Comment 19

[Simon Montagu :smontagu]

Thanks for fixing this, Masayuki, and sorry for making such a careless mistake.