Closed Bug 92623 Opened 23 years ago Closed 22 years ago

WRMB: Adjust the nsCSSParser quirks

Categories

(Core :: CSS Parsing and Computation, defect, P1)

Product:
Core
Component:
CSS Parsing and Computation
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.4

People

(Reporter: harishd, Assigned: attinasi)

References

Details

(Keywords: compat, topembed, Whiteboard: [checked in on trunk and branch], [still see on linux])

Attachments

(6 files)

test case with font-size
284 bytes, text/html
Details
test case screen shot with IE 5.5sp1/win2k
20.25 KB, image/jpeg
Details
test case screen shot with mozilla 2001-72622/win2k
17.26 KB, image/jpeg
Details
Patch: surgically removes the font-size antiquirk and ungets the token for invalid font weights
1.88 KB, patch
Details | Diff | Splinter Review
atachment 43871 testcase screenshot viewed in Linux
18.39 KB, image/png
Details
testcase with fontsize 40, 40pt, and 40px
271 bytes, text/html
Details 
Testcase:

<html>
 <head>
  <style>
   .one { color: red; font: 20; }
   .two { color: blue; font: 20px; }
   .three { color: green; font 20pt; }
  </style>
 </head>
 <body> 
  <p class="one">SIZE 20 </p>
  <p class="two">SIZE 20px </p>
  <p class="three">SIZE 20pt </p>
 </body>
</html>

Load the above test case and see if you can distinguish between px & pt sizes.
Harish: your testcase is invalid. The 'font' property takes a length *and* a
family name. Neither is optional. In your testcase, we ignore all three 'font'
declarations and use the author's prefs for each of them.

Marking INVALID, whatever page that comes from should go to Evang. (Harish: See 
also Bugscape WRMB 8021.)
Status: NEW → RESOLVED
Closed: 23 years ago
Keywords: compat
Resolution: --- → INVALID
The test case with screen shots show that in the event that a font-size is
incorrectly specified without a unit, Mozilla and IE's behavior is different. IE
defaults to px units while I am not sure what Mozilla is doing. Changing my font
preferences for serif or sans-serif has no effect on the size of the text
specified without a unit.

This is a candiate for a browser side compatibility fix since many people forget
the units on font sizes.

I think this should be reconsidered.
Ok. There are two "bugs" here.

The first is that we don't UngetToken() in ParseFontWeight() if the number is
not a valid keyword.

The second is that we specifically disable the unitless numbers quirk for the
'font-size' property. This anti-quirk is Nav4.x compatible but not IE 
compatible, so it causes some problems on some sites.

I will attach a patch which "fixes" both of these issues.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Summary: px & pt sizes are rendered alike. → Adjust the nsCSSParser quirks
cc'ing a bunch of folks. Reviews please, I'm not very familiar with this code 
so I don't know what I've broken...
Assignee: attinasi → ian
Status: REOPENED → NEW
OS: Windows NT → All
Priority: -- → P1
QA Contact: ian → bclary
Hardware: PC → All
Whiteboard: [fix in hand]
Target Milestone: --- → mozilla0.9.4
My memory was that we had to disable it at least for the 'font' shorthand
because otherwise the size could be confused with the weight.
Bug 23461 seems to be where the antiquirk was added; I propose removing it
because IE does not have that antiquirk.

The font weight issue should not be a problem, either. If the unitless font 
size matches a font weight, then it will be interpreted as a weight, which is
fine since font sizes of 100 pixels are rare, particularly in 'font' property
declarations... With my patch, we act the same as IE on:

   http://www.hixie.ch/tests/adhoc/css/parsing/compatability/002.html
Ian, your patch looks correct to me. [s]r=attinasi
reviewed, tested, approved, r=pierre
Keywords: topembed
Could I ask for a check-in buddy to check this into the trunk? Marc? Pierre?
It would be much appreciated.
Checked in on the trunk.
Oops... sorry Ian: I had the change in my tree but I needed to do an update 
before checking in.

Thanks Jag.  Marked Fixed.
Status: NEW → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Reopening due to topembed keyword. This needs to be checked in on the 092 branch
if all goes well on the trunk.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [fix in hand] → [checked in on trunk]
adding marek and chofmann
Summary: Adjust the nsCSSParser quirks → WRMB: Adjust the nsCSSParser quirks
attinasi: Could you check this into the branch as well? Cheers.
Reassigning to attinasi to stop Marek from bugging me. I have no branch tree
and I'm not pulling one before reformatting my disk. :-)
Assignee: ian → attinasi
Status: REOPENED → NEW
Ian: attinasi is not here today, I think, and I heard that topembed bugs had to 
be checked in today at the latest.  Try to find a check-in buddy with a branch (I 
don't have one myself, sorry).

Note that we need a checkin buddy before the power is out at 8pm...

Pierre et al, while it is important to get this checked in, we don't need to
rush it today (i.e., Friday)
OK, I'll land it when it opens.
Status: NEW → ASSIGNED
Checekd into branch too now. Cheers, Hixie
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Whiteboard: [checked in on trunk] → [checked in on trunk and branch]
*** Bug 97522 has been marked as a duplicate of this bug. ***
attachment 43871 [details] working fine on win32 and macOS ----
win2000 : 2002-05-23-08 trunk build
win2000 : 2002-05-22-1.0.0 branch build
macOS 9.1 : 2002-05-21-1.0.0 branch build 
macOS 9.1 : 2002-05-21-03 trunk build


but the bug still seems to be there on the linux builds----
redhat Linux 7.3 : 2002-05-23-1.0.0 branch build
redhat Linux 7.3 : 2002-05-23-07trunk build

I suppose the bug  now lives as a linux only bug. reopening bug for further 
evaluation.
Status: RESOLVED → REOPENED
OS: All → Linux
Hardware: All → PC
Resolution: FIXED → ---
Whiteboard: [checked in on trunk and branch] → [checked in on trunk and branch], [still see on linux]
Could you describe the symptoms that you see on Linux that make you think this 
bug still exists.  (Is it a problem with the fonts that you have installed?)

The fontsize of 20, 20pt and 20px looks the same in Linux, whereas the same
sizes when displayed in win32 and MacOS are clearly distinguished.
Try modifying the testcase so it says 40, and see what happens.  Maybe the
default font size on your Linux box is already 20px.
I see no difference in the fontsize with this testcase too. same situation as
with fontsize 20.
Wait -- the point of this bug is that 40 and 40px *are* supposed to look the
same.  40px vs. 40pt should differ based on the logical resolution.
I guess you are right, David. Probably my machine resolution needs to be updated.

I checked the the testcases on a different linux box, and the fontsizes look
fine on that. Based on that, I am marking the bug fixed and verified. Thanks !!!
Status: REOPENED → RESOLVED
Closed: 23 years ago → 22 years ago
Resolution: --- → FIXED
verified fixed
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: