Bug 5564

Summary: 'font' shorthand parsing should be more tolerant in quirks mode
Product: WebKit Reporter: David Latapie <david>
Component: CSSAssignee: Dave Hyatt <hyatt>
Status: RESOLVED INVALID    
Severity: Normal CC: ahmad.saleem792, ap, emacemac7, hyatt, joost, koivisto, mmaxfield, simon.fraser
Priority: P2 Keywords: InRadar
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: https://bugreport.apple.com/cgi-bin/WebObjects/RadarWeb.woa/4/wo/6CE7G0t5bR5F5dJs5yTnt0/3.33.26.0.3
Attachments:
Description Flags
First attempt
darin: review-
Now with testcase
none
Now with expected results
darin: review-
Now uses Arial
none
Improved patch
none
Safari 15.5 matches other browsers none

Description David Latapie 2005-10-30 10:00:38 PST
Copied from bugreport.apple.com to make it publicly avaiblable

On the following code, Test 1 and test 2 should both appear italic but only test 2 does

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/
xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="fr-fr">
<head>
	<style type="text/css">
	.test1	{font:italic 1.5em}
	.test2	{font:italic 1.5em helvetica}
	</style>
</head>

<body>
<p class="test1">Test 1</p>
<p class="test2">Test 2</p>
</body>
</html>
Comment 1 Nicholas Shanks 2006-01-20 07:01:53 PST
I did a quick test with the following HTML. What is observed (417.8 & ToT) is stated after the equals for 
each case. It should be self explanatory:
(n.b. I am aware that many of these do not adhere to the specs, but this was in compatibility mode 
anyway)

<p style="font: 24pt italic;">24pt italic = 24pt</p>
<p style="font: 24pt italic Gentium;">24pt italic Gentium = 24pt</p>
<p style="font: 24pt italic 'Gentium';">24pt italic 'Gentium' = 24pt Gentium</p>
<p style="font: italic 24pt;">italic 24pt = Fails</p>
<p style="font: italic 24pt Gentium;">italic 24pt Gentium = 24pt Gentium Italic</p>
<p style="font: italic 24pt 'Gentium';">italic 24pt 'Gentium' = 24pt Gentium Italic</p>
<p style="font: Gentium 24pt italic;">Gentium 24pt italic = fails</p>
<p style="font: 'Gentium' 24pt italic;">'Gentium' 24pt italic = fails</p>
<p style="font: Gentium italic 24pt;">Gentium italic 24pt = fails</p>
<p style="font: 'Gentium' italic 24pt;">'Gentium' italic 24pt = fails</p>
Comment 2 Nicholas Shanks 2006-02-21 12:18:56 PST
See also bug #3303.
Comment 3 Rob Buis 2006-05-13 09:43:00 PDT
AFAICS the original testcase uses the font shorthand in an illegal way.
Also Firefox renders it the same as Safari does. So while bug 3303 seems
valid, I question this bug report. Opinions?
Cheers,

Rob.
Comment 4 David Latapie 2006-05-13 11:57:07 PDT
(In reply to comment #3)
> AFAICS the original testcase uses the font shorthand in an illegal way.

It looks correct to me. http://www.w3.org/TR/CSS21/fonts.html#font-shorthand

Why do you think it is incorrect?
Comment 5 Alexey Proskuryakov 2006-05-13 13:52:47 PDT
(In reply to comment #4)
> Why do you think it is incorrect?

The spec says that <'font-size'> and <'font-family'> are not optional (if we forget about system fonts, of course). See <http://www.w3.org/TR/CSS21/about.html#q7> for the description of the syntax used.

Safari behavior matches Firefox; however, WinIE allows this syntax in quirks mode (it behaves correctly in strict mode). Please note that the presence of an XML declaration triggers quirks mode in WinIE.
Comment 6 David Latapie 2006-05-13 16:31:35 PDT
OK then. I close the bug as INVALID.
Comment 7 Alexey Proskuryakov 2006-05-14 00:34:31 PDT
Joost, what is your opinion about this issue? 
Comment 8 Joost de Valk (AlthA) 2006-05-14 01:31:19 PDT
Hmm, though it's not fully standards compliant, i think we should render this as IE does. It's quite clear what people mean when they write this down ".test1	{font:italic 1.5em;}" so, i think this should be reopened. Hyatt, what;s your opinion?
Comment 9 Dave Hyatt 2006-05-14 15:19:47 PDT
Being more permissive in quirks mode to match WinIE seems reasonable, so yeah, I agree that we should keep this open.
Comment 10 Rob Buis 2006-05-30 13:32:41 PDT
Created attachment 8606 [details]
First attempt

My first stab at this, works like winIE on the test snippet. I can make a
testcase if the code gets okayed, maybe based on the test snippet?
Cheers,

Rob.
Comment 11 Darin Adler 2006-05-31 14:54:44 PDT
Comment on attachment 8606 [details]
First attempt

Looks good. We should write test cases not just for the bug fixed, but ideally to cover each different "if" statement that's changed.
Comment 12 Rob Buis 2006-06-01 02:23:35 PDT
Created attachment 8636 [details]
Now with testcase
Comment 13 Darin Adler 2006-06-01 13:21:25 PDT
Comment on attachment 8636 [details]
Now with testcase

Patch needs to include expected results too.
Comment 14 Rob Buis 2006-06-01 13:57:40 PDT
Created attachment 8651 [details]
Now with expected results

Hi Darin,

Totally forgot about that :) Now included, let me know if anything else
needs improvement....
Cheers,

Rob.
Comment 15 Darin Adler 2006-06-01 17:03:11 PDT
Comment on attachment 8651 [details]
Now with expected results

I don't think that a font named "Gentium" is standard with Mac OS X, so I think these tests require installing a non-standard font. Can we make a version of this test that works with some more-standard font?
Comment 16 David Kilzer (:ddkilzer) 2006-06-02 02:11:19 PDT
This bug appears to be in Radar given the original URL, but the specific Radar bug number is not listed here.  Poster, could you list the Radar bug number?
Comment 17 Rob Buis 2006-06-02 02:26:16 PDT
Created attachment 8660 [details]
Now uses Arial

Apart from it now using Arial I also fixed a typo, pointed out by
nickshanks.
Cheers,

Rob.
Comment 18 Rob Buis 2006-06-02 13:20:23 PDT
Created attachment 8668 [details]
Improved patch

I included the wrong expected results in the patch, this one fixes it.
Cheers,

Rob.
Comment 19 Maciej Stachowiak 2006-06-04 21:18:36 PDT
Comment on attachment 8668 [details]
Improved patch

r=me
Comment 20 David Kilzer (:ddkilzer) 2006-06-05 20:08:58 PDT
Committed revision 14740.
Comment 21 David Harrison 2006-07-19 07:14:19 PDT
Hyatt backed out this fix in r15490, with the comment:

"Back out the fix for 5564, since it turns out font:x-small; is a pretty
prominent IE-specific CSS hack.  Because Web sites rely on IE's incorrect
font parsing as a means of also correcting IE's incorrect font size rules.

This fixes Yahoo.com."
Comment 22 Joost de Valk (AlthA) 2006-07-19 07:34:55 PDT
Reopening per the previous comment, this should probably have a new patch.
Comment 23 Alexey Proskuryakov 2006-07-24 08:59:37 PDT
Comment on attachment 8668 [details]
Improved patch

Clearing the review flag, so that this doesn't show up in commit queue.
Comment 24 Ahmad Saleem 2022-06-01 05:36:34 PDT
Created attachment 459924 [details]
Safari 15.5 matches other browsers

I changed the test case from Comment 1 into JSFiddle as below:

Link - https://jsfiddle.net/z0gyL7nu/

Later, I tested it across browsers and as shown in the attached picture, all works same.

Should we mark this as Closed or "RESOLVED INVALID"?
Comment 25 Sam Sneddon [:gsnedders] 2022-07-01 12:00:15 PDT
https://quirks.spec.whatwg.org/ has no quirk for this, so yeah, let's assume we aren't ever going to match WinIE.
Comment 26 Myles C. Maxfield 2022-07-05 21:42:27 PDT
🆒