CSS 2.1 failure: vertical-align-boxes-001 fails
authorrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 14 Jul 2012 08:57:27 +0000 (08:57 +0000)
committerrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 14 Jul 2012 08:57:27 +0000 (08:57 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/css2.1/20110323/vertical-align-boxes-001-expected.html [new file with mode: 0644]
LayoutTests/css2.1/20110323/vertical-align-boxes-001.htm [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RootInlineBox.cpp

index d3811c2..2291a57 100644 (file)
@@ -1,3 +1,15 @@
+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>
 
         Unreviewed GTK gardening, marking http/tests/misc/webtiming-origins.html as flaky.
diff --git a/LayoutTests/css2.1/20110323/vertical-align-boxes-001-expected.html b/LayoutTests/css2.1/20110323/vertical-align-boxes-001-expected.html
new file mode 100644 (file)
index 0000000..faacd24
--- /dev/null
@@ -0,0 +1,39 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+ <head>
+  <title>CSS Test: Reference Result</title>
+  <style type="text/css">
+    .test {
+        height: 2em;
+        font: 40px/60px Ahem; color: blue;
+    }
+
+    #control {
+      height: 40px;
+      margin-bottom: -18px;
+    }
+    #control2 {
+      height: 40px;
+      width: 15px;
+      margin-bottom: -18px;
+    }
+
+    img {
+      background: blue;
+    }
+  </style>
+ </head>
+ <body>
+  <p>There should be a single blue rectangle with perfectly straight sides below.</p>
+  <div class="test">
+    <img id="control" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="control2" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="control2" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="control2" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="control2" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="control2" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="control2" src="support/swatch-blue.png" alt="FAIL">
+  </div>
+
+ </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/css2.1/20110323/vertical-align-boxes-001.htm b/LayoutTests/css2.1/20110323/vertical-align-boxes-001.htm
new file mode 100644 (file)
index 0000000..6f6c5fc
--- /dev/null
@@ -0,0 +1,74 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+ <head>
+  <title>CSS Test: Vertical Alignment boxes: replaced elements</title>
+  <link rel="author" title="Elika J. Etemad" href="http://fantasai.inkedblade.net/contact">
+  <link rel="help" href="http://www.w3.org/TR/CSS21/visudet.html#line-height">
+  <meta name="flags" content="ahem image">
+  <meta name="assert" content="Vertical alignment aligns the margin box of inline replaced elements.">
+  <style type="text/css">
+    .test {
+        height: 2em;
+        font: 40px/60px Ahem; color: blue;
+    }
+
+    #control {
+      height: 40px;
+      margin-bottom: -18px;
+    }
+    #length {
+      vertical-align: -28px;
+      padding-top: 15px;
+      border-top: 10px solid;
+      margin: 10px 0;
+    }
+    #percent {
+      vertical-align: 50%;
+      padding-bottom: 15px;
+      border-bottom: 10px solid;
+      margin-bottom: -48px;
+    }
+    #text-bottom {
+      vertical-align: text-bottom;
+      padding-bottom: 10px;
+      border-bottom: 15px solid;
+      margin-bottom: -10px;
+    }
+    #text-top {
+      vertical-align: text-top;
+      padding-top: 15px;
+      border-top: 10px solid;
+      margin-top: 10px;
+    }
+    #middle {
+      vertical-align: middle;
+      padding-top: 15px;
+      border-top: 10px solid;
+      margin-bottom: -28px;
+    }
+    #baseline {
+      vertical-align: baseline;
+      padding-bottom: 10px;
+      border-bottom: 15px solid;
+      margin-bottom: -18px;
+    }
+
+    img {
+      background: blue;
+    }
+  </style>
+ </head>
+ <body>
+  <p>There should be a single blue rectangle with perfectly straight sides below.</p>
+  <div class="test">
+    <img id="control" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="baseline" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="middle" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="text-top" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="text-bottom" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="percent" src="support/swatch-blue.png" alt="FAIL"><!--
+ --><img id="length" src="support/swatch-blue.png" alt="FAIL">
+  </div>
+
+ </body>
+</html>
\ No newline at end of file
index 1dbb66d..8268a78 100644 (file)
@@ -1,3 +1,23 @@
+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>
 
         Iterating backwards over HTMLCollection is O(n^2)
index e410be0..675b071 100644 (file)
@@ -883,8 +883,15 @@ LayoutUnit RootInlineBox::verticalPositionForBox(InlineBox* box, VerticalPositio
                 verticalPosition -= (renderer->lineHeight(firstLine, lineDirection) - renderer->baselinePosition(baselineType(), firstLine, lineDirection));
         } 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());
+        }
     }
 
     // Store the cached value.