- Revision
- 125229
- Author
- [email protected]
- Date
- 2012-08-09 18:05:56 -0700 (Thu, 09 Aug 2012)
Log Message
REGRESSION(r117339): cell in block-level table in inline-block are aligned with their last line box
https://bugs.webkit.org/show_bug.cgi?id=91137
Reviewed by Tony Chang.
Source/WebCore:
Fixed the code to properly ignore any inline-table's baseline for the purpose of 'inline-block' baseline computation.
See http://lists.w3.org/Archives/Public/www-style/2012Jul/0721.html about the discussion.
While touching the code, properly fixed the table baseline logic that was wrong because it wrongly piggy-backed
on the inline-block baseline logic.
Tests: fast/table/anonymous-table-no-baseline-align.html
fast/table/inline-table-in-inline-block-last-baseline-align.html
* rendering/RenderTable.cpp:
(WebCore::RenderTable::baselinePosition):
Overrode this function: this is needed as RenderBlock assumes that isReplaced() == true
means it is handling an inline-block, not a table.
(WebCore::RenderTable::lastLineBoxBaseline):
Table are ignored for the purpose of inline-block baseline determination.
(WebCore::RenderTable::firstLineBoxBaseline):
Inlined the code from getLineBoxBaseline. Added some comments to explain
why this code works as expected in the 'table' case.
* rendering/RenderTable.h:
Added baselinePosition.
LayoutTests:
* fast/table/anonymous-table-no-baseline-align-expected.html: Added.
* fast/table/anonymous-table-no-baseline-align.html: Added.
* fast/table/inline-table-in-inline-block-last-baseline-align-expected.html: Added.
* fast/table/inline-table-in-inline-block-last-baseline-align.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (125228 => 125229)
--- trunk/LayoutTests/ChangeLog 2012-08-10 00:58:49 UTC (rev 125228)
+++ trunk/LayoutTests/ChangeLog 2012-08-10 01:05:56 UTC (rev 125229)
@@ -1,3 +1,15 @@
+2012-08-09 Julien Chaffraix <[email protected]>
+
+ REGRESSION(r117339): cell in block-level table in inline-block are aligned with their last line box
+ https://bugs.webkit.org/show_bug.cgi?id=91137
+
+ Reviewed by Tony Chang.
+
+ * fast/table/anonymous-table-no-baseline-align-expected.html: Added.
+ * fast/table/anonymous-table-no-baseline-align.html: Added.
+ * fast/table/inline-table-in-inline-block-last-baseline-align-expected.html: Added.
+ * fast/table/inline-table-in-inline-block-last-baseline-align.html: Added.
+
2012-08-09 Mikhail Pozdnyakov <[email protected]>
[WK2] Unskip tests requiring layoutTestController.dumpResourceResponseMIMETypes
Added: trunk/LayoutTests/fast/table/anonymous-table-no-baseline-align-expected.html (0 => 125229)
--- trunk/LayoutTests/fast/table/anonymous-table-no-baseline-align-expected.html (rev 0)
+++ trunk/LayoutTests/fast/table/anonymous-table-no-baseline-align-expected.html 2012-08-10 01:05:56 UTC (rev 125229)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.wrapper {
+ height: 100px;
+ width: 120px;
+ display: inline-block;
+ background-color: green;
+}
+</style>
+</head>
+<body>
+ <p>Bug <a href="" REGRESSION(r117339): cell in block-level table in inline-block are aligned with their last line box</p>
+ <p>This test checks that an anonymous 'table' doesn't contribute its baseline when determining an inline-block baseline.</p>
+ <p>The 2 green squares should be aligned below.</p>
+ <div class="wrapper"></div>
+ <div class="wrapper"></div>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/anonymous-table-no-baseline-align-expected.html
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/fast/table/anonymous-table-no-baseline-align.html (0 => 125229)
--- trunk/LayoutTests/fast/table/anonymous-table-no-baseline-align.html (rev 0)
+++ trunk/LayoutTests/fast/table/anonymous-table-no-baseline-align.html 2012-08-10 01:05:56 UTC (rev 125229)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.wrapper {
+ height: 100px;
+ width: 120px;
+ display: inline-block;
+ background-color: green;
+}
+.cell {
+ display: table-cell;
+ vertical-align: middle;
+ color: green;
+}
+</style>
+</head>
+<body>
+ <p>Bug <a href="" REGRESSION(r117339): cell in block-level table in inline-block are aligned with their last line box</p>
+ <p>This test checks that an anonymous 'table' doesn't contribute its baseline when determining an inline-block baseline.</p>
+ <p>The 2 green squares should be aligned below.</p>
+ <div class="wrapper">
+ <div class="cell"> </div>
+ </div>
+ <div class="wrapper">
+ <div class="cell"> <br> <br></div>
+ </div>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/anonymous-table-no-baseline-align.html
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align-expected.html (0 => 125229)
--- trunk/LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align-expected.html (rev 0)
+++ trunk/LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align-expected.html 2012-08-10 01:05:56 UTC (rev 125229)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.wrapper {
+ height: 100px;
+ display: inline-block;
+ color: blue;
+ font: 16px Ahem;
+}
+</style>
+</head>
+<body>
+ <p>Bug <a href="" REGRESSION(r117339): cell in block-level table in inline-block are aligned with their last line box</p>
+ <p>This test checks that an 'inline-table' contributes its baseline to determine an inline-block baseline.</p>
+ <p>The 2 blue texts should be aligned below.</p>
+ <div class="wrapper">First</div>
+ <div class="wrapper"> <br> <br>Second</div>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align-expected.html
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align.html (0 => 125229)
--- trunk/LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align.html (rev 0)
+++ trunk/LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align.html 2012-08-10 01:05:56 UTC (rev 125229)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.wrapper {
+ height: 100px;
+ display: inline-block;
+ font: 16px Ahem;
+}
+
+table {
+ display: inline-table;
+ border-spacing: 0px;
+}
+
+td {
+ padding: 0px;
+ vertical-align: middle;
+ color: blue;
+}
+</style>
+</head>
+<body>
+ <p>Bug <a href="" REGRESSION(r117339): cell in block-level table in inline-block are aligned with their last line box</p>
+ <p>This test checks that an 'inline-table' contributes its baseline to determine an inline-block baseline.</p>
+ <p>The 2 blue texts should be aligned below.</p>
+ <div class="wrapper">
+ <table>
+ <td>First</td>
+ </table>
+ </div>
+ <div class="wrapper">
+ <table>
+ <td> <br> <br>Second</td>
+ </table>
+ </div>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align.html
___________________________________________________________________
Added: svn:executable
Modified: trunk/Source/WebCore/ChangeLog (125228 => 125229)
--- trunk/Source/WebCore/ChangeLog 2012-08-10 00:58:49 UTC (rev 125228)
+++ trunk/Source/WebCore/ChangeLog 2012-08-10 01:05:56 UTC (rev 125229)
@@ -1,3 +1,34 @@
+2012-08-09 Julien Chaffraix <[email protected]>
+
+ REGRESSION(r117339): cell in block-level table in inline-block are aligned with their last line box
+ https://bugs.webkit.org/show_bug.cgi?id=91137
+
+ Reviewed by Tony Chang.
+
+ Fixed the code to properly ignore any inline-table's baseline for the purpose of 'inline-block' baseline computation.
+ See http://lists.w3.org/Archives/Public/www-style/2012Jul/0721.html about the discussion.
+
+ While touching the code, properly fixed the table baseline logic that was wrong because it wrongly piggy-backed
+ on the inline-block baseline logic.
+
+ Tests: fast/table/anonymous-table-no-baseline-align.html
+ fast/table/inline-table-in-inline-block-last-baseline-align.html
+
+ * rendering/RenderTable.cpp:
+ (WebCore::RenderTable::baselinePosition):
+ Overrode this function: this is needed as RenderBlock assumes that isReplaced() == true
+ means it is handling an inline-block, not a table.
+
+ (WebCore::RenderTable::lastLineBoxBaseline):
+ Table are ignored for the purpose of inline-block baseline determination.
+
+ (WebCore::RenderTable::firstLineBoxBaseline):
+ Inlined the code from getLineBoxBaseline. Added some comments to explain
+ why this code works as expected in the 'table' case.
+
+ * rendering/RenderTable.h:
+ Added baselinePosition.
+
2012-08-09 Adam Barth <[email protected]>
Sandboxing view source documents is ineffective
Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (125228 => 125229)
--- trunk/Source/WebCore/rendering/RenderTable.cpp 2012-08-10 00:58:49 UTC (rev 125228)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp 2012-08-10 01:05:56 UTC (rev 125229)
@@ -1206,16 +1206,33 @@
{
}
-enum LineBox { FirstLineBox, LastLineBox };
+LayoutUnit RenderTable::baselinePosition(FontBaseline baselineType, bool firstLine, LineDirectionMode direction, LinePositionMode linePositionMode) const
+{
+ LayoutUnit baseline = firstLineBoxBaseline();
+ if (baseline != -1)
+ return baseline;
-static LayoutUnit getLineBoxBaseline(const RenderTable* table, LineBox lineBox)
+ return RenderBox::baselinePosition(baselineType, firstLine, direction, linePositionMode);
+}
+
+LayoutUnit RenderTable::lastLineBoxBaseline() const
{
- if (table->isWritingModeRoot())
+ // Tables don't contribute their baseline towards the computation of an inline-block's baseline.
+ return -1;
+}
+
+LayoutUnit RenderTable::firstLineBoxBaseline() const
+{
+ // The baseline of a 'table' is the same as the 'inline-table' baseline per CSS 3 Flexbox (CSS 2.1
+ // doesn't define the baseline of a 'table' only an 'inline-table').
+ // This is also needed to properly determine the baseline of a cell if it has a table child.
+
+ if (isWritingModeRoot())
return -1;
- table->recalcSectionsIfNeeded();
+ recalcSectionsIfNeeded();
- const RenderTableSection* topNonEmptySection = table->topNonEmptySection();
+ const RenderTableSection* topNonEmptySection = this->topNonEmptySection();
if (!topNonEmptySection)
return -1;
@@ -1223,26 +1240,10 @@
if (baseline > 0)
return topNonEmptySection->logicalTop() + baseline;
- // The 'first' linebox baseline in a table in the absence of any text in the first section
- // is the top of the table.
- if (lineBox == FirstLineBox)
- return topNonEmptySection->logicalTop();
-
- // The 'last' linebox baseline in a table is the baseline of text in the first
- // cell in the first row/section, so if there is no text do not return a baseline.
+ // FIXME: A table row always has a baseline per CSS 2.1. Will this return the right value?
return -1;
}
-LayoutUnit RenderTable::lastLineBoxBaseline() const
-{
- return getLineBoxBaseline(this, LastLineBox);
-}
-
-LayoutUnit RenderTable::firstLineBoxBaseline() const
-{
- return getLineBoxBaseline(this, FirstLineBox);
-}
-
LayoutRect RenderTable::overflowClipRect(const LayoutPoint& location, RenderRegion* region, OverlayScrollbarSizeRelevancy relevancy)
{
LayoutRect rect = RenderBlock::overflowClipRect(location, region, relevancy);
Modified: trunk/Source/WebCore/rendering/RenderTable.h (125228 => 125229)
--- trunk/Source/WebCore/rendering/RenderTable.h 2012-08-10 00:58:49 UTC (rev 125228)
+++ trunk/Source/WebCore/rendering/RenderTable.h 2012-08-10 01:05:56 UTC (rev 125229)
@@ -251,6 +251,7 @@
virtual void computePreferredLogicalWidths();
virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestPoint& pointInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) OVERRIDE;
+ virtual LayoutUnit baselinePosition(FontBaseline, bool firstLine, LineDirectionMode, LinePositionMode = PositionOnContainingLine) const OVERRIDE;
virtual LayoutUnit firstLineBoxBaseline() const OVERRIDE;
virtual LayoutUnit lastLineBoxBaseline() const OVERRIDE;
