Bug 5564 - 'font' shorthand parsing should be more tolerant in quirks mode
Summary: 'font' shorthand parsing should be more tolerant in quirks mode
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Dave Hyatt
URL: https://bugreport.apple.com/cgi-bin/W...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2005-10-30 10:00 PST by David Latapie
Modified: 2010-06-10 16:55 PDT (History)
4 users (show)

See Also:


Attachments
First attempt (2.05 KB, patch)
2006-05-30 13:32 PDT, Rob Buis
darin: review-
Details | Formatted Diff | Diff
Now with testcase (5.53 KB, patch)
2006-06-01 02:23 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Now with expected results (10.27 KB, patch)
2006-06-01 13:57 PDT, Rob Buis
darin: review-
Details | Formatted Diff | Diff
Now uses Arial (10.21 KB, patch)
2006-06-02 02:26 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Improved patch (10.15 KB, patch)
2006-06-02 13:20 PDT, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.