108012 - text measurment on win xp have problem with unicode while word-spacing and letter-spacing is present

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Frank Tang]

this is really a win XP bug. Not sure can be reproduce on win2000 or not.
We cannot reproduce this on NT.
We find this bug while we look at buscape bug 9852. We probably can word around
9852 by change css
but the real problem is in GFX
While word-spacing and letter-spacing is present. The Unicode measurement
behavior have problem on winxp

Comment 1

[Frank Tang]

Attachment: test case 1 for word-spacing

Comment 2

[Frank Tang]

Attachment: test case 2 for letter-spacing

Comment 3

[Frank Tang]

hum... it looks like the above test cases does NOT show the problem on XP.
Please ignore them.

Comment 4

[Frank Tang]

also look at bugscape 10647

Comment 5

[Shanjian Li]

reassign to myself.

Comment 6

[Shanjian Li]

Attachment: minimal testcase to show the problem

Comment 7

[Shanjian Li]

In function "nsRenderingContextWin :: GetWidth", there are lines,
#ifdef IBMBIDI
  WORD charType=0;
  if (::GetStringTypeW(CT_CTYPE3, &ch, 1, &charType) && (charType & C3_DIACRITIC) 
	    && !(charType & C3_ALPHA)) {
//    aWidth = 0;
    GetWidth(buf, 1, aWidth, aFontID);
    aWidth *=-1;
    return NS_OK;
  }
#endif
  return GetWidth(buf, 1, aWidth, aFontID);

Very unfortunately, japanese character u30fc (katakana-hiragana prolonged sound
mark) is tested as a diacritic and not a alphbet, and its width is multiplied by
negative one, that cause the overlapping. 

This problem only happens when u30fc is being measured by itself, that's why
letter-spacing need to be present. 

Comment 8

[Shanjian Li]

add simon to cc list

Comment 9

[Shanjian Li]

Attachment: proposed patch

I talked to simon today, and he told me the whole bunch of bidi hack can/should
be removed.

Comment 10

[Simon Montagu :smontagu]

Just to fill in some background: that IBMBIDI block was part of an attempted
hack to fix bug 60546, but it wasn't very successful and I had intended to
remove it before submitting the diffs for the big bidi checkins last March, and
work on a better fix later. That this portion remained when the rest was blown
away was purely an oversight on my part.

Removing it will have no negative side effects on Bidi rendering. 

Comment 11

[Shanjian Li]

simon, can you put r= there if you believe my patch is fine? thanks!

Comment 12

[Simon Montagu :smontagu]

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

r=smontagu

Comment 13

[Shanjian Li]

Marc, could you sr? 

Comment 14

[Marc Attinasi]

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

sr=attinasi - go!

Comment 15

[Shanjian Li]

fix checked in.