120343 - Several languages on Unicode pages rendering with unwanted squares

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Tracy Walker [:tracy]]

seen on windows only...comercial build windows 2002-01-16-06-trunk

-Goto above Unicode page
notice that several languages are rendering with the unwanted squares

Comment 1

[Simon Montagu :smontagu]

If the languages affected are Arabic, Hebrew and Yiddish, this is a dupe of bug
119659. I have the fix for that in my local tree, and the whole page displays OK.

Marking as dupe, but please feel free to reopen if other languages are being
mis-rendered.

*** This bug has been marked as a duplicate of 119659 ***

Comment 2

[Roy Yokoyama]

This happens with at least with Korean as well.

Comment 3

[Roy Yokoyama]

cc'ing shanjian.

Comment 4

[Roy Yokoyama]

shanjian: do you think it may be because of your checkin?
http://bonsai.mozilla.org/showcheckins.cgi?person=shanjian%25netscape.com&treeid=SeaMonkey

Comment 5

[Roy Yokoyama]

Verifed 2002-01-15-09-trunk build is ok.

Comment 6

[Asa Dotzler [:asa]]

Roy, so this is no longer a problem? Should it be resolved or is it still a
smoketest blocker?

Comment 7

[Roy Yokoyama]

Yesterday's was ok (2002-01-15-09-trunk)
Today's is broken. (2002-01-16-06-trunk)
It's a smoketest broker.

Comment 8

[Roy Yokoyama]

shanjian: I rolled back your change (see Comment #3) in my locale tree
and I no longer see the problem.
I am assigning this bug to you. Please have a look ASAP. thanks

Comment 9

[Roy Yokoyama]

shanjian: I noticed that behaviour of text selection 
is also affected by your change.  When you have a fix,
please check the text selection as well.  thanks

Comment 10

[Shanjian Li]

I understood the problem, working on a fix now.

Comment 11

[Shanjian Li]

Attachment: proposed patch

Comment 12

[Shanjian Li]

marc, chris,
This a blocker, need your sr fast.

Comment 13

[Shanjian Li]

frank, rbs, roy,
Could any of you give code review? thanks.

Comment 14

[Simon Montagu :smontagu]

Comment on attachment 65297 [details] [diff] [review]
proposed patch

r=smontagu

Comment 15

[Chris Waterson]

I don't understand this code very well, but the original code (r3.150)
pre-incremented currChar before calling LocateFont:

  while (++currChar < lastChar) {
    nextFont = LocateFont(aDC, *currChar, count);
    if (nextFont != currFont) {


And this version of the code doesn't. Sorry, but someone will have to hold my
hand here: why we don't need to pre-increment anymore? (I'm assuming that this
is a regression from the checking for bug 118000...)

Comment 16

[Shanjian Li]

The reason is that we need to advance 2 int16 for a surrogate pair, and 1 for 
non surrogate code points. So I decide to advance it later. 

Comment 17

[Chris Waterson]

Okay, shanjian walked me through this. Turns out that each preceding loop leaves
|currChar| properly setup for the loop that follow it, so there's no need to
pre-increment here. sr=waterson

Comment 18

[Shanjian Li]

fix checked in. 

Comment 19

[rbs]

I was just having a look at the initial change in bug 118000, and saw this:

   //This if block is mean to speedup the process in normal situation, when
   //most characters can be found in first font
   if (currFont == mLoadedFonts[0]) {
-    while (++currChar < lastChar && currFont->HasGlyph(*currChar)) ;
+    while (currChar < lastChar && (currFont->HasGlyph(*currChar)))
+      ++currChar;

and wondered why there isn't the +1 or +2 issue there? (note: my tree is not yet
up-to-date and I don't have these changes in full picture yet).

Comment 20

[Shanjian Li]

rbs, 
I intentionally leave it that way for performance reason. You know that code 
is a fast track and it is not really necessary. Skip surrogate test will surely
keep the speed for non-surrogate characters. For surrogate pair, since part of 
a surrogate pair should not find glyph in any of the font, it will fall back 
to code follows. 

Comment 21

[Shanjian Li]

*** Bug 120274 has been marked as a duplicate of this bug. ***

Comment 22

[Tracy Walker [:tracy]]

verified fixed on windows commercial build 2002-01-17-06-trunk