Summary: | 'font' shorthand parsing should be more tolerant in quirks mode | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Latapie <david> | ||||||||||||||
Component: | CSS | Assignee: | 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
David Latapie
2005-10-30 10:00:38 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> 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. (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? (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. OK then. I close the bug as INVALID. Joost, what is your opinion about this issue? 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? Being more permissive in quirks mode to match WinIE seems reasonable, so yeah, I agree that we should keep this open. 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 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.
Created attachment 8636 [details]
Now with testcase
Comment on attachment 8636 [details]
Now with testcase
Patch needs to include expected results too.
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 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?
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? Created attachment 8660 [details]
Now uses Arial
Apart from it now using Arial I also fixed a typo, pointed out by
nickshanks.
Cheers,
Rob.
Created attachment 8668 [details]
Improved patch
I included the wrong expected results in the patch, this one fixes it.
Cheers,
Rob.
Comment on attachment 8668 [details]
Improved patch
r=me
Committed revision 14740. 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." Reopening per the previous comment, this should probably have a new patch. Comment on attachment 8668 [details]
Improved patch
Clearing the review flag, so that this doesn't show up in commit queue.
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"? https://quirks.spec.whatwg.org/ has no quirk for this, so yeah, let's assume we aren't ever going to match WinIE. 🆒 |