Reviewed by Dave Hyatt.
authorrwlbuis <rwlbuis@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 May 2007 14:00:20 +0000 (14:00 +0000)
committerrwlbuis <rwlbuis@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 May 2007 14:00:20 +0000 (14:00 +0000)
        http://bugs.webkit.org/show_bug.cgi?id=13174
        line-height in font shorthand does not override a previously stated line-height property

        Make sure line-height is calculated against definite font-size and
        uses the last set line-height, ie. directly or through font shorthand.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@21667 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
LayoutTests/ChangeLog
LayoutTests/css1/font_properties/font-expected.checksum
LayoutTests/css1/font_properties/font-expected.png
LayoutTests/css1/font_properties/font-expected.txt
LayoutTests/fast/css/line-height-font-order-expected.checksum [new file with mode: 0644]
LayoutTests/fast/css/line-height-font-order-expected.png [new file with mode: 0644]
LayoutTests/fast/css/line-height-font-order-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/line-height-font-order.html [new file with mode: 0644]
LayoutTests/tables/mozilla/bugs/bug83786-expected.checksum
LayoutTests/tables/mozilla/bugs/bug83786-expected.png
LayoutTests/tables/mozilla/bugs/bug83786-expected.txt
WebCore/ChangeLog
WebCore/css/cssstyleselector.cpp
WebCore/css/cssstyleselector.h

index fab4e30..5d87bd7 100644 (file)
@@ -1,5 +1,26 @@
 2007-05-23  Rob Buis  <buis@kde.org>
 
+        Reviewed by Dave Hyatt.
+
+        Testcase for:
+        http://bugs.webkit.org/show_bug.cgi?id=13174
+        line-height in font shorthand does not override a previously stated line-height property
+
+        The two changed results are improvements.
+
+        * css1/font_properties/font-expected.checksum:
+        * css1/font_properties/font-expected.png:
+        * css1/font_properties/font-expected.txt:
+        * fast/css/line-height-font-order-expected.checksum: Added.
+        * fast/css/line-height-font-order-expected.png: Added.
+        * fast/css/line-height-font-order-expected.txt: Added.
+        * fast/css/line-height-font-order.html: Added.
+        * tables/mozilla/bugs/bug83786-expected.checksum:
+        * tables/mozilla/bugs/bug83786-expected.png:
+        * tables/mozilla/bugs/bug83786-expected.txt:
+
+2007-05-23  Rob Buis  <buis@kde.org>
+
         Reviewed by Darin.
 
         Test for:
index 7ee56d7..9f6018a 100644 (file)
@@ -1 +1 @@
-f21461ab5ad341769216660e3ed62726
\ No newline at end of file
+94dc4c29f7704f467e1f720a3f190cf5
\ No newline at end of file
index 858ce0a..6f6f29d 100644 (file)
Binary files a/LayoutTests/css1/font_properties/font-expected.png and b/LayoutTests/css1/font_properties/font-expected.png differ
index 19ae00d..d665990 100644 (file)
@@ -1,8 +1,8 @@
-layer at (0,0) size 785x4302
+layer at (0,0) size 785x4266
   RenderView at (0,0) size 785x600
-layer at (0,0) size 785x4302
-  RenderBlock {HTML} at (0,0) size 785x4302
-    RenderBody {BODY} at (8,8) size 769x4286 [bgcolor=#CCCCCC]
+layer at (0,0) size 785x4266
+  RenderBlock {HTML} at (0,0) size 785x4266
+    RenderBody {BODY} at (8,8) size 769x4250 [bgcolor=#CCCCCC]
       RenderBlock {P} at (0,0) size 769x14
         RenderText {#text} at (0,0) size 264x14
           text run at (0,0) width 264: "The style declarations which apply to the text below are:"
@@ -119,18 +119,18 @@ layer at (0,0) size 785x4302
             text run at (0,124) width 750: "54px, respectively). The text should have a silver background. The background color has been set"
             text run at (0,178) width 679: "on an inline element and should therefore only cover the text, not the interline spacing."
         RenderText {#text} at (0,0) size 0x0
-      RenderTable {TABLE} at (0,1821) size 769x2465 [border: (1px outset #808080)]
-        RenderTableSection {TBODY} at (1,1) size 767x2463
+      RenderTable {TABLE} at (0,1821) size 769x2429 [border: (1px outset #808080)]
+        RenderTableSection {TBODY} at (1,1) size 767x2427
           RenderTableRow {TR} at (0,0) size 767x26
             RenderTableCell {TD} at (0,0) size 767x26 [bgcolor=#C0C0C0] [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=2]
               RenderInline {STRONG} at (0,0) size 163x18
                 RenderText {#text} at (4,4) size 163x18
                   text run at (4,4) width 163: "TABLE Testing Section"
-          RenderTableRow {TR} at (0,26) size 767x2437
-            RenderTableCell {TD} at (0,1231) size 12x26 [bgcolor=#C0C0C0] [border: (1px inset #808080)] [r=1 c=0 rs=1 cs=1]
+          RenderTableRow {TR} at (0,26) size 767x2401
+            RenderTableCell {TD} at (0,1213) size 12x26 [bgcolor=#C0C0C0] [border: (1px inset #808080)] [r=1 c=0 rs=1 cs=1]
               RenderText {#text} at (4,4) size 4x18
                 text run at (4,4) width 4: " "
-            RenderTableCell {TD} at (12,26) size 755x2437 [border: (1px inset #808080)] [r=1 c=1 rs=1 cs=1]
+            RenderTableCell {TD} at (12,26) size 755x2401 [border: (1px inset #808080)] [r=1 c=1 rs=1 cs=1]
               RenderBlock {P} at (4,4) size 747x36
                 RenderText {#text} at (0,0) size 732x36
                   text run at (0,0) width 564: "This element is unstyled, and should inherit a font-size of 12px from the BODY element. "
@@ -185,14 +185,14 @@ layer at (0,0) size 785x4302
                   text run at (0,238) width 147: "respectively). "
                   text run at (147,238) width 559: "Extra text is included for the purposes of testing this"
                   text run at (0,310) width 175: "more effectively."
-              RenderBlock {P} at (4,1127) size 747x192
-                RenderText {#text} at (0,12) size 740x167
-                  text run at (0,12) width 740: "This element should be in a monospace font, italicized and small caps, with a weight"
-                  text run at (0,60) width 88: "of 900. "
-                  text run at (88,60) width 620: "Its font-size should be 150% the base font size, and its line-height"
-                  text run at (0,108) width 716: "should be 2em, or twice the element's font size (18px and 36px, respectively)."
-                  text run at (0,156) width 636: "Extra text is included for the purposes of testing this more effectively."
-              RenderBlock {P} at (4,1343) size 747x384
+              RenderBlock {P} at (4,1127) size 747x156
+                RenderText {#text} at (0,8) size 740x140
+                  text run at (0,8) width 740: "This element should be in a monospace font, italicized and small caps, with a weight"
+                  text run at (0,47) width 88: "of 900. "
+                  text run at (88,47) width 620: "Its font-size should be 150% the base font size, and its line-height"
+                  text run at (0,86) width 716: "should be 2em, or twice the element's font size (18px and 36px, respectively)."
+                  text run at (0,125) width 636: "Extra text is included for the purposes of testing this more effectively."
+              RenderBlock {P} at (4,1307) size 747x384
                 RenderText {#text} at (0,34) size 739x316
                   text run at (0,34) width 702: "This element should be in a sans-serif font, italicized and small caps,"
                   text run at (0,130) width 225: "with a weight of 500. "
@@ -200,7 +200,7 @@ layer at (0,0) size 785x4302
                   text run at (0,226) width 433: "or 18px, and its line-height should be 1in. "
                   text run at (433,226) width 306: "Extra text is included for the"
                   text run at (0,322) width 428: "purposes of testing this more effectively."
-              RenderBlock {P} at (4,1751) size 747x144
+              RenderBlock {P} at (4,1715) size 747x144
                 RenderText {#text} at (0,7) size 730x129
                   text run at (0,7) width 730: "This element should be in a sans-serif font, oblique and not small-caps, with a weight"
                   text run at (0,43) width 62: "of 700. "
@@ -208,13 +208,13 @@ layer at (0,0) size 785x4302
                   text run at (0,79) width 176: "element's font size). "
                   text run at (176,79) width 500: "Extra text is included for the purposes of testing this more"
                   text run at (0,115) width 93: "effectively."
-              RenderBlock {P} at (4,1913) size 747x64
+              RenderBlock {P} at (4,1877) size 747x64
                 RenderText {#text} at (0,8) size 714x47
                   text run at (0,8) width 378: "This element should be in a sans-serif font, with a weight of 400. "
                   text run at (378,8) width 336: "Its font-size should be 80% of 12px, or 9.6px, and its line-"
                   text run at (0,40) width 235: "height shoud be 2.5 times that, or 24px. "
                   text run at (235,40) width 405: "Extra text is included for the purposes of testing this more effectively."
-              RenderBlock {P} at (4,2001) size 747x432
+              RenderBlock {P} at (4,1965) size 747x432
                 RenderInline {SPAN} at (0,0) size 716x388 [bgcolor=#C0C0C0]
                   RenderText {#text} at (0,22) size 716x388
                     text run at (0,22) width 702: "This element should be in a sans-serif font, italicized and small caps,"
diff --git a/LayoutTests/fast/css/line-height-font-order-expected.checksum b/LayoutTests/fast/css/line-height-font-order-expected.checksum
new file mode 100644 (file)
index 0000000..bfc710b
--- /dev/null
@@ -0,0 +1 @@
+2e2203a5ee65ffa382f924a92c656537
\ No newline at end of file
diff --git a/LayoutTests/fast/css/line-height-font-order-expected.png b/LayoutTests/fast/css/line-height-font-order-expected.png
new file mode 100644 (file)
index 0000000..1de9ca8
Binary files /dev/null and b/LayoutTests/fast/css/line-height-font-order-expected.png differ
diff --git a/LayoutTests/fast/css/line-height-font-order-expected.txt b/LayoutTests/fast/css/line-height-font-order-expected.txt
new file mode 100644 (file)
index 0000000..5b533e6
--- /dev/null
@@ -0,0 +1,21 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x196
+  RenderBlock {HTML} at (0,0) size 800x196
+    RenderBody {BODY} at (8,15) size 784x166
+      RenderBlock {P} at (0,0) size 784x75
+        RenderText {#text} at (0,29) size 94x17
+          text run at (0,29) width 94: "This tests bug "
+        RenderInline {A} at (0,0) size 651x17 [color=#0000EE]
+          RenderText {#text} at (94,29) size 651x17
+            text run at (94,29) width 651: "Bug 13174: line-height in font shorthand does not override a previously stated line-height property"
+        RenderText {#text} at (745,29) size 4x17
+          text run at (745,29) width 4: "."
+      RenderBlock {P} at (0,90) size 784x76
+        RenderText {#text} at (0,29) size 129x17
+          text run at (0,29) width 129: "This text should be "
+        RenderInline {CODE} at (0,0) size 189x17
+          RenderText {#text} at (129,30) size 189x17
+            text run at (129,30) width 189: "font:15px/5em Georgia"
+        RenderText {#text} at (318,29) size 4x17
+          text run at (318,29) width 4: "."
diff --git a/LayoutTests/fast/css/line-height-font-order.html b/LayoutTests/fast/css/line-height-font-order.html
new file mode 100644 (file)
index 0000000..4a52988
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
+        "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head>
+<title>Shorthand line-height bug</title>
+<style>
+p {line-height:1em;}
+p {font:15px/5em Georgia;}
+</style>
+
+</head>
+<body>
+    <p>This tests bug <a href="http://bugs.webkit.org/show_bug.cgi?id=13174">Bug 13174: line-height in font shorthand does not override a previously stated line-height property</a>.</p>
+    <p>This text should be <code>font:15px/5em Georgia</code>.</p>
+</body>
+</html>
+
index f004b50..b687a46 100644 (file)
@@ -1 +1 @@
-cc23795c1de6c488f74253cfdf0722ab
\ No newline at end of file
+3d9b2c7ad5469e072272f4381358d743
\ No newline at end of file
index 7e46b79..216b9a7 100644 (file)
Binary files a/LayoutTests/tables/mozilla/bugs/bug83786-expected.png and b/LayoutTests/tables/mozilla/bugs/bug83786-expected.png differ
index c09fb43..0a1af81 100644 (file)
@@ -3,24 +3,24 @@ layer at (0,0) size 800x600
 layer at (0,0) size 800x136
   RenderBlock {HTML} at (0,0) size 800x136
     RenderBody {BODY} at (0,0) size 800x136
-layer at (0,0) size 266x136 clip at (3,3) size 260x130 scrollHeight 176
+layer at (0,0) size 266x136 clip at (3,3) size 260x130 scrollHeight 144
   RenderBlock {DIV} at (0,0) size 266x136 [border: (3px solid #008000)]
-    RenderTable {TABLE} at (16,3) size 64x176
-      RenderTableSection {TBODY} at (0,0) size 64x176
-        RenderTableRow {TR} at (0,0) size 64x176
-          RenderTableCell {TD} at (0,0) size 64x176 [r=0 c=0 rs=1 cs=1]
-            RenderText {#text} at (0,0) size 64x159
-              text run at (0,0) width 64: "This____"
-              text run at (0,16) width 64: "text____"
-              text run at (0,32) width 64: "should__"
-              text run at (0,48) width 64: "be______"
-              text run at (0,64) width 64: "wrapped,"
-              text run at (0,80) width 64: "one_____"
-              text run at (0,96) width 64: "word____"
-              text run at (0,112) width 64: "to______"
-              text run at (0,128) width 64: "a_______"
-              text run at (0,144) width 64: "line.___"
+    RenderTable {TABLE} at (16,3) size 64x143
+      RenderTableSection {TBODY} at (0,0) size 64x143
+        RenderTableRow {TR} at (0,0) size 64x143
+          RenderTableCell {TD} at (0,0) size 64x143 [r=0 c=0 rs=1 cs=1]
+            RenderText {#text} at (0,-1) size 64x132
+              text run at (0,-1) width 64: "This____"
+              text run at (0,12) width 64: "text____"
+              text run at (0,25) width 64: "should__"
+              text run at (0,38) width 64: "be______"
+              text run at (0,51) width 64: "wrapped,"
+              text run at (0,64) width 64: "one_____"
+              text run at (0,77) width 64: "word____"
+              text run at (0,90) width 64: "to______"
+              text run at (0,103) width 64: "a_______"
+              text run at (0,116) width 64: "line.___"
             RenderInline {SPAN} at (0,0) size 56x14 [color=#FFFF00] [bgcolor=#FF0000]
-              RenderText {#text} at (0,161) size 56x14
-                text run at (0,161) width 56: "_FAIL!__"
+              RenderText {#text} at (0,130) size 56x14
+                text run at (0,130) width 56: "_FAIL!__"
             RenderText {#text} at (0,0) size 0x0
index 1b3c999..c93a4f0 100644 (file)
@@ -1,3 +1,20 @@
+2007-05-23  Rob Buis  <buis@kde.org>
+
+        Reviewed by Dave Hyatt.
+
+        http://bugs.webkit.org/show_bug.cgi?id=13174
+        line-height in font shorthand does not override a previously stated line-height property
+
+        Make sure line-height is calculated against definite font-size and
+        uses the last set line-height, ie. directly or through font shorthand.
+
+        * css/cssstyleselector.cpp:
+        (WebCore::CSSStyleSelector::styleForElement):
+        (WebCore::CSSStyleSelector::pseudoStyleForElement):
+        (WebCore::CSSStyleSelector::applyDeclarations):
+        (WebCore::CSSStyleSelector::applyProperty):
+        * css/cssstyleselector.h:
+
 2007-05-23  Lars Knoll <lars@trolltech.com>
 
         Reviewed by Zack and Valgrind
index 121a83a..8170655 100644 (file)
@@ -879,6 +879,7 @@ RenderStyle* CSSStyleSelector::styleForElement(Element* e, RenderStyle* defaultP
     // high-priority properties first, i.e., those properties that other properties depend on.
     // The order is (1) high-priority not important, (2) high-priority important, (3) normal not important
     // and (4) normal important.
+    m_lineHeightValue = 0;
     applyDeclarations(true, false, 0, m_matchedDecls.size() - 1);
     if (!resolveForRootDefault) {
         applyDeclarations(true, true, firstAuthorRule, lastAuthorRule);
@@ -889,7 +890,11 @@ RenderStyle* CSSStyleSelector::styleForElement(Element* e, RenderStyle* defaultP
     // If our font got dirtied, go ahead and update it now.
     if (fontDirty)
         updateFont();
-    
+
+    // Line-height is set when we are sure we decided on the font-size
+    if (m_lineHeightValue)
+        applyProperty(CSS_PROP_LINE_HEIGHT, m_lineHeightValue);
+
     // Now do the normal priority UA properties.
     applyDeclarations(false, false, firstUARule, lastUARule);
     
@@ -949,6 +954,7 @@ RenderStyle* CSSStyleSelector::pseudoStyleForElement(RenderStyle::PseudoId pseud
         parentStyle = style;
     style->noninherited_flags._styleType = pseudoStyle;
     
+    m_lineHeightValue = 0;
     // High-priority properties.
     applyDeclarations(true, false, 0, m_matchedDecls.size() - 1);
     applyDeclarations(true, true, firstAuthorRule, lastAuthorRule);
@@ -958,6 +964,10 @@ RenderStyle* CSSStyleSelector::pseudoStyleForElement(RenderStyle::PseudoId pseud
     // If our font got dirtied, go ahead and update it now.
     if (fontDirty)
         updateFont();
+
+    // Line-height is set when we are sure we decided on the font-size
+    if (m_lineHeightValue)
+        applyProperty(CSS_PROP_LINE_HEIGHT, m_lineHeightValue);
     
     // Now do the normal priority properties.
     applyDeclarations(false, false, firstUARule, lastUARule);
@@ -1751,6 +1761,10 @@ void CSSStyleSelector::applyDeclarations(bool applyFirst, bool isImportant,
             if (isImportant == current.isImportant()) {
                 bool first;
                 switch(current.id()) {
+                    case CSS_PROP_LINE_HEIGHT:
+                        m_lineHeightValue = current.value();
+                        first = !applyFirst; // we apply line-height later
+                        break;
                     case CSS_PROP_COLOR:
                     case CSS_PROP_DIRECTION:
                     case CSS_PROP_DISPLAY:
@@ -1769,7 +1783,6 @@ void CSSStyleSelector::applyDeclarations(bool applyFirst, bool isImportant,
                         first = false;
                         break;
                 }
-                
                 if (first == applyFirst)
                     applyProperty(current.id(), current.value());
             }
@@ -3515,16 +3528,19 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
         if (isInherit) {
             FontDescription fontDescription = parentStyle->fontDescription();
             style->setLineHeight(parentStyle->lineHeight());
+            m_lineHeightValue = 0;
             if (style->setFontDescription(fontDescription))
                 fontDirty = true;
         } else if (isInitial) {
             FontDescription fontDescription;
             fontDescription.setGenericFamily(FontDescription::StandardFamily);
             style->setLineHeight(RenderStyle::initialLineHeight());
+            m_lineHeightValue = 0;
             if (style->setFontDescription(fontDescription))
                 fontDirty = true;
         } else if (primitiveValue) {
             style->setLineHeight(RenderStyle::initialLineHeight());
+            m_lineHeightValue = 0;
             FontDescription fontDescription;
             theme()->systemFont(primitiveValue->getIdent(), fontDescription);
             // Double-check and see if the theme did anything.  If not, don't bother updating the font.
@@ -3544,13 +3560,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
             applyProperty(CSS_PROP_FONT_WEIGHT, font->weight.get());
             applyProperty(CSS_PROP_FONT_SIZE, font->size.get());
 
-            // Line-height can depend on font().pixelSize(), so we have to update the font
-            // before we evaluate line-height, e.g., font: 1em/1em.  FIXME: Still not
-            // good enough: style="font:1em/1em; font-size:36px" should have a line-height of 36px.
-            if (fontDirty)
-                CSSStyleSelector::style->font().update();
-            
-            applyProperty(CSS_PROP_LINE_HEIGHT, font->lineHeight.get());
+            m_lineHeightValue = font->lineHeight.get();
             applyProperty(CSS_PROP_FONT_FAMILY, font->family.get());
         }
         return;
index 836bb39..90699fd 100644 (file)
@@ -224,6 +224,7 @@ class StyledElement;
         RenderStyle::PseudoId pseudoStyle;
         FrameView *view;
         Frame *frame;
+        CSSValue* m_lineHeightValue;
         const Settings *settings;
         bool fontDirty;
         bool isXMLDoc;