larger doesn't work when font-size <= 10px

VERIFIED FIXED

Status

()

P1
normal
VERIFIED FIXED
20 years ago
2 years ago

People

(Reporter: dbaron, Assigned: pierre)

Tracking

({css1, fonts, testcase})

Trunk
css1, fonts, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fix described in bug, URL)

Attachments

(2 attachments)

DESCRIPTION:  The larger keyword for the CSS1 font-size property doesn't work
when the parent element's font size is less than or equal to 10px.  This
includes the keyword xx-small.

STEPS TO REPRODUCE:  Load attached test case.

ACTUAL RESULTS:  The font only gets bigger on the second and fifth lines.  The
1st, 3d, and 4th lines have the same font size throughout.

EXPECTED RESULTS:  On every line, each word should be bigger than the one before
it.

DOES NOT WORK CORRECTLY ON:
 * Linux, viewer, 1999-08-24-08-M10
 * Windows 95, apprunner, 1999-08-24-09-M10

WORKS CORRECTLY ON:
 * Windows IE 4.0
The threshhold seems to vary slightly by platform, actually.  The 11px case (5th
line) doesn't work in Windows, but it does in Linux.  Probably 12px or 13px
would work... since x-small does.

Updated

20 years ago
Assignee: troy → peterl
Component: Layout → Style System

Comment 3

20 years ago
I think this must be a style system. Layout just uses the info we get from the
style system

Comment 4

20 years ago
Pushing off non-beta 1 issues

Updated

20 years ago
QA Contact: petersen → chrisd
(Assignee)

Comment 5

19 years ago
Reassigning peterl's bugs to myself.
(Assignee)

Comment 6

19 years ago
Accepting peterl's bugs that have a Target Milestone

Comment 7

19 years ago
The problem looks like it is for fonts of 9pt and below not 10px and below.
That would account for the system differences, different px=>pt conversions.

With default values (12pt Times) the cut off in the html table code comes out
at 9.24pt or 184.8 twips.

Look at
http://lxr.mozilla.org/seamonkey/source/layout/html/style/src/nsStyleUtil.cpp#1
57

you seek slowly downward till you find a returned font size that is smaller
than the current one then go back up one step. Bing! always the same size for
fonts smaller than 9.24pt.  Change the greaterthan to a equals test and you
fix this bug, I think.

Hope it helps.

Comment 8

19 years ago
*sigh* it is line 157

Also, you should use >= rather than == just in case the math gets "jumpy" and
skips a value.  There is a pow() in there and index is it's exponent. As index
gets higher/lower it will REALLY swing that value. for outlandish text sizes it
might get buggy with ==.

Comment 9

19 years ago
Verified Fix under linux against m11 codebase
(Assignee)

Comment 10

19 years ago
Pushing my M15 bugs to M16
Keywords: css1
Migrating from {css1} to css1 keyword. The {css1}, {css2}, {css3} and {css-moz}
radars should now be considered deprecated in favour of keywords.
I am *really* sorry about the spam...
Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!
Keywords: testcase
(Assignee)

Updated

19 years ago
Priority: P3 → P1
Summary: {css1}larger doesn't work when font-size <= 10px → {font}{css1}larger doesn't work when font-size <= 10px
Target Milestone: M16 → M14

Updated

19 years ago
Summary: {font}{css1}larger doesn't work when font-size <= 10px → {font}larger doesn't work when font-size <= 10px
Pierre, you might wish to have a look at this while you do all the other font
stuff, especially since there is a fix described in the bug.
Summary: {font}larger doesn't work when font-size <= 10px → larger doesn't work when font-size <= 10px {font}
Keywords: fonts
Summary: larger doesn't work when font-size <= 10px {font} → larger doesn't work when font-size <= 10px

Comment 14

19 years ago
Moving crufty m14-m15 bugs out to m16 for proper triage.
Target Milestone: M14 → M16
(Assignee)

Comment 15

19 years ago
The algorithm has changed a while back. Larger/smaller acts on the HTML sizes 
which are constrained to [0 - 7] but within that range, they do work for any 
size. This behavior is identical to IE's.

Marked M18. A solution would be to fall back to the old algorithm when the font 
size is out of range, even if we are in strict mode.
Target Milestone: M16 → M18
(Assignee)

Updated

19 years ago
Target Milestone: M18 → Future
(Assignee)

Comment 16

19 years ago
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another known 
resource will be working on this bug, or if it blocks your work in some way -- 
please attach your concern to the bug for reconsideration. 

Comment 17

19 years ago
Um, so what needs to be done here?
I'm going to attach a patch that does a bit of code cleanup.

Justification for loss of future: css1
relnote2: if we don't fully support css we need to say so.
nsbeta3: css1
Assignee: pierre → nobody
Status: ASSIGNED → NEW
Keywords: nsbeta3, patch, relnote2
Whiteboard: [TESTCASE] fix described in bug → fix described in bug
Target Milestone: Future → ---

Comment 18

19 years ago
Created attachment 11455 [details] [diff] [review]
patch reduce possible rounding error; r=disttsc@bart.nl
Marc, Pierre: could one of you also review Josh's patch and (if it passes muster 
with you guys) help get it checked in, a=brendan@mozilla.org?  Thanks.

/be

Comment 20

19 years ago
This bug is not nsbeta2+ and should not be checked in. We are trying to minimize 
changes and increase stability, not fix every small bug.

Also, reassigning back to Pierre: why was it remvoed from his queue and sent to 
'nobody' in the first place?
Assignee: nobody → pierre
Netscape employees should not be fixing bugs that are not nsbeta2+ .  That
restriction does not apply to others.  See the top of tinderbox.
phil@netscape.com has stated this too, most recently in mozilla.seamonkey:
you're a mozilla.org module owner (you happen to work for netscape.com).  Part
of your job as module owner is to take good patches when they're ready to go
into the trunk (even if they're "too risky" for your employers' product).  Yes,
module owners may have to serve two masters; life's rough.  staff@mozilla.org
will have to take steps if an owner freezes out good patches for too long (where
good patches are defined by the owner and peers in the entire community, based
on code review and technical merits).

In this case, I don't think anyone has argued that this patch is "too risky",
just that "we're trying to stabilize."  Notwithstanding that stability effort,
could we not get this patch checked in?  If not, can we have some specific risk
analysis for why not?

/be

Comment 23

19 years ago
The patch does not look risky. It is very localized. I have applied the patch 
and the testcase looks great. Thanks for the patch, timeless! Would you like me 
to check it in, or do you have checkin privs?

Comment 24

19 years ago
I do not have cvs privs, i should get them soon, but i haven't gotten around to 
doing the paperwork. Please commit, thank you.

Updated

19 years ago
Status: NEW → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 25

19 years ago
Patch checked in, thanks again timeless!

Comment 26

19 years ago
With the following builds:
Win: 7_18_20
Mac: 7_19_12
Linux: 7_20_09
Using the testcase provided, verifying bug fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.