Bug 13174

Summary: line-height in font shorthand does not override a previously stated line-height property
Product: WebKit Reporter: Dan Richman <dan>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gavin.sharp
Priority: P2 Keywords: HasReduction
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
URL: http://level39.com/font-shorthand/
Attachments:
Description Flags
Testcase
none
First attempt
hyatt: review-
Different approach hyatt: review+

Description Dan Richman 2007-03-23 15:15:44 PDT
If you set a line-height value for a block of text, and then later on in your style sheet you try to declare a font shorthand property that contains a line-height value, it will not override the previously stated line-height property.

A workaround is to simple re-declare a regular line-height property in addition to the font shorthand property.

Please compare the testcase in other browsers to see proper implementation.
Comment 1 Dan Richman 2007-03-23 15:16:55 PDT
Created attachment 13787 [details]
Testcase
Comment 2 Rob Buis 2007-05-21 01:19:01 PDT
Created attachment 14641 [details]
First  attempt

This should do it. It can be cleaned up a bit more I think, but I hope the approach is right.
Cheers,

Rob.
Comment 3 Dave Hyatt 2007-05-21 01:33:31 PDT
Comment on attachment 14641 [details]
First  attempt

Per conversation on IRC, I suggested just always making line-height be mapped in as one of the first properties like font.

However instead of going into the RenderStyle in the switch statement it would just be cached as a member variable on the style selector.

After the style has been determined (e.g., around the time adjustRenderStyle is going to get called), that line height value (if it exists) should be mapped in for real.
Comment 4 Rob Buis 2007-05-22 02:09:36 PDT
Created attachment 14650 [details]
Different approach

As discussed with Hyatt on IRC, this is probably a better approach.
Cheers,

Rob.
Comment 5 Dave Hyatt 2007-05-23 01:10:46 PDT
Comment on attachment 14650 [details]
Different approach

r=me
Comment 6 David Kilzer (:ddkilzer) 2007-05-23 17:21:14 PDT
Confirmed by existence of patches.

Comment 7 Sam Weinig 2007-05-24 17:39:56 PDT
Landed in r21667.