Changeset 122663 in webkit

Timestamp:

Jul 14, 2012 1:57:27 AM (12 years ago)

Author:

robert@webkit.org

Message:

CSS 2.1 failure: vertical-align-boxes-001 fails
​https://bugs.webkit.org/show_bug.cgi?id=90626

Reviewed by Eric Seidel.

Source/WebCore:

Tests: css2.1/20110323/vertical-align-boxes-001.htm

A percentage value vertical-align is always a percentage of the actual line-height rather than
the margin box per ​http://www.w3.org/TR/CSS21/visudet.html#propdef-vertical-align: 'Percentages:
refer to the 'line-height' of the element itself'. Confusingly, RenderBox::lineheight() is a
shorthand into the dimensions of the margin box for replaced elements in the other vertical-align
cases, i.e. where it's the margin box that's relevant rather than the 'line-height'. So rather than patch RenderBox's
lineHeight() to somehow consider the percentage cases, just give percentage vertical-align the full computedLineHeight()
rather than lineHeight()'s margin box.

• rendering/RootInlineBox.cpp:

(WebCore::RootInlineBox::verticalPositionForBox):

LayoutTests:

• css2.1/20110323/vertical-align-boxes-001-expected.html: Added.
• css2.1/20110323/vertical-align-boxes-001.htm: Added. This patch fixes the 'percentage' case in this test so that it calculates 50% of the line-height of 60px rather than 50% of the img's margin-box of -8px.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css2.1/20110323/vertical-align-boxes-001-expected.html (added)
• LayoutTests/css2.1/20110323/vertical-align-boxes-001.htm (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RootInlineBox.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-07-05  Robert Hogan  <robert@webkit.org>
+
+        CSS 2.1 failure: vertical-align-boxes-001 fails
+        https://bugs.webkit.org/show_bug.cgi?id=90626
+
+        Reviewed by Eric Seidel.
+
+        * css2.1/20110323/vertical-align-boxes-001-expected.html: Added.
+        * css2.1/20110323/vertical-align-boxes-001.htm: Added.
+          This patch fixes the 'percentage' case in this test so that it calculates 50% of the
+          line-height of 60px rather than 50% of the img's margin-box of -8px.
+
 2012-07-14  Zan Dobersek  <zandobersek@gmail.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-05  Robert Hogan  <robert@webkit.org>
+
+        CSS 2.1 failure: vertical-align-boxes-001 fails
+        https://bugs.webkit.org/show_bug.cgi?id=90626
+
+        Reviewed by Eric Seidel.
+
+        Tests: css2.1/20110323/vertical-align-boxes-001.htm
+
+        A percentage value vertical-align is always a percentage of the actual line-height rather than
+        the margin box per http://www.w3.org/TR/CSS21/visudet.html#propdef-vertical-align: 'Percentages: 
+        refer to the 'line-height' of the element itself'.  Confusingly, RenderBox::lineheight() is a
+        shorthand into the dimensions of the margin box for replaced elements in the other vertical-align
+        cases, i.e. where it's the margin box that's relevant rather than the 'line-height'. So rather than patch RenderBox's
+        lineHeight() to somehow consider the percentage cases, just give percentage vertical-align the full computedLineHeight()
+        rather than lineHeight()'s margin box.
+
+        * rendering/RootInlineBox.cpp:
+        (WebCore::RootInlineBox::verticalPositionForBox):
+
 2012-07-13  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebCore/rendering/RootInlineBox.cpp

@@ -884 +884 @@
         } else if (verticalAlign == BASELINE_MIDDLE)
             verticalPosition += -renderer->lineHeight(firstLine, lineDirection) / 2 + renderer->baselinePosition(baselineType(), firstLine, lineDirection);
-        else if (verticalAlign == LENGTH)
-            verticalPosition -= valueForLength(renderer->style()->verticalAlignLength(), renderer->lineHeight(firstLine, lineDirection), renderer->view());
+        else if (verticalAlign == LENGTH) {
+            LayoutUnit lineHeight;
+            //Per http://www.w3.org/TR/CSS21/visudet.html#propdef-vertical-align: 'Percentages: refer to the 'line-height' of the element itself'.
+            if (renderer->style()->verticalAlignLength().isPercent())
+                lineHeight = renderer->style()->computedLineHeight();
+            else
+                lineHeight = renderer->lineHeight(firstLine, lineDirection);
+            verticalPosition -= valueForLength(renderer->style()->verticalAlignLength(), lineHeight, renderer->view());
+        }
     }