Bug 140801

Summary: Import fast/css/font-shorthand-line-height.html layout test from Blink
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: CSSAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, kling, koivisto
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: https://src.chromium.org/viewvc/blink?view=rev&revision=184346
Bug Depends on:    
Bug Blocks: 140577    
Attachments:
Description Flags
Patch none

Description Chris Dumez 2015-01-22 16:37:42 PST
Import fast/css/font-shorthand-line-height.html layout test from Blink to get better test coverage for 'line-height' font shorthands cascades.

I am working on expanding the font shorthand at parsing time (like other properties) via Bug 140577 and I want to make sure I don't break line-height in the process.
Comment 1 Chris Dumez 2015-01-22 16:40:12 PST
Created attachment 245184 [details]
Patch
Comment 2 Andreas Kling 2015-01-22 17:35:11 PST
Comment on attachment 245184 [details]
Patch

Nice!
Comment 3 WebKit Commit Bot 2015-01-22 18:17:31 PST
Comment on attachment 245184 [details]
Patch

Clearing flags on attachment: 245184

Committed r178976: <http://trac.webkit.org/changeset/178976>
Comment 4 WebKit Commit Bot 2015-01-22 18:17:39 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Chris Dumez 2015-01-22 20:59:50 PST
Comment on attachment 245184 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=245184&action=review

> LayoutTests/fast/css/font-shorthand-line-height.html:27
> +    shouldBe("lineHeight(system_font_2)", "'100px'");

If I understand the specification correctly (http://dev.w3.org/csswg/css-fonts/#font-prop), this is wrong:
1. "All subproperties of the ‘font’ property are first reset to their initial values. Then, those properties that are given explicit values in the ‘font’ shorthand are set to those values."
2. "system fonts may only be set as a whole; that is, the font family, size, weight, style, etc. are all set at the same time."

So the line-height here should have been reset and the result should be "normal", not "100px". Chrome gives "normal" here. I will take a look to fix this but please let me know if you think I misunderstood the spec.