353872 - font-size-adjust broken on windows

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Doug Wright]

See url for hixie's testcases from 2001

Comment 1

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

Attachment: Patch rv1.0

fix.

Comment 2

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

Attachment: Patch rv1.1

We should check whether the mAdjustedSize is not zero.

Comment 3

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

Stuart:

Would you review this too? This is very simple.

Comment 4

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

checked-in.

Comment 5

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070105 Minefield/3.0a2pre ID:2007010512 [cairo]

http://hixie.ch/tests/adhoc/css/fonts/size-adjust/004.xml
http://hixie.ch/tests/adhoc/css/fonts/size-adjust/005.xml
http://hixie.ch/tests/adhoc/css/fonts/size-adjust/006.xml
http://hixie.ch/tests/adhoc/css/fonts/size-adjust/009.xml

still fail.

I don't know if you want to reopen this bug or open another one ?

Comment 6

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

(In reply to comment #5)
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070105
> Minefield/3.0a2pre ID:2007010512 [cairo]
> 
> http://hixie.ch/tests/adhoc/css/fonts/size-adjust/004.xml
> http://hixie.ch/tests/adhoc/css/fonts/size-adjust/005.xml
> http://hixie.ch/tests/adhoc/css/fonts/size-adjust/006.xml
> http://hixie.ch/tests/adhoc/css/fonts/size-adjust/009.xml
> 
> still fail.

Wow, thank you for the report. I see. The font-size is rounded to zero at calculating. I'll attach the patch.

Comment 7

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

(In reply to comment #6)
> I see. The font-size is rounded to zero at calculating.
> 

Please ignore this comment.

Comment 8

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

(In reply to comment #5)
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070105
> Minefield/3.0a2pre ID:2007010512 [cairo]
> 
> http://hixie.ch/tests/adhoc/css/fonts/size-adjust/004.xml
> http://hixie.ch/tests/adhoc/css/fonts/size-adjust/005.xml
> http://hixie.ch/tests/adhoc/css/fonts/size-adjust/006.xml
> http://hixie.ch/tests/adhoc/css/fonts/size-adjust/009.xml
> 
> still fail.
> 
> I don't know if you want to reopen this bug or open another one ?
> 

Oops, I may have a misread the spec of CSS2. Should the all line-height of the testcases be calculated from actual value of the line (i.e., the adjusted size by font-size-adjust)? If so, the old gfx has same problem. (I believed that the old gfx is correct, but it's wrong?)

Comment 9

[Peter van der Woude [:Peter6]]

(In reply to comment #8)

> Oops, I may have a misread the spec of CSS2. Should the all line-height of the
> testcases be calculated from actual value of the line (i.e., the adjusted size
> by font-size-adjust)? If so, the old gfx has same problem. (I believed that the
> old gfx is correct, but it's wrong?)

Don't ask me, I just wanted to verify this bug and noted that a part of the testcases you gave as reference still failed.

Comment 10

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

dbaron:

Should the line-height value be computed from adjusted font size? or from computed value of font-size? I believed that it is computed from font-size, but Peter said that the patch still fails textcase 4, 5, 6 and 9. The line-heights are calculated from the computed value of font-size. This behavior is same as old gfx (win32). But on old gfx, the 'ex' is calculated from adjusted font size... What is a correct result??
# I think that I need to re-create the patch for linux in bug 363410.

Comment 11

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

I'm waiting the answer from dbaron.

But if the line-heights should be calculating from adjusted font size, it may not be this bug. Because in nsRuleNode.cpp, http://lxr.mozilla.org/mozilla/source/layout/style/nsRuleNode.cpp#219
the font-size value is used for the calculating except 'ex'.

Comment 12

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

I filed bug 366138 for the 'issue' for testcases 4, 5, 6 and 9.

Please check the bug for them.

marking to FIXED.

Comment 13

[Steve England [:stevee]]

Could someone please take a look at bug 413137 and triage it appropriately?