Changeset 125229 in webkit

Timestamp:

Aug 9, 2012 6:05:56 PM (12 years ago)

Author:

jchaffraix@webkit.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/table/anonymous-table-no-baseline-align-expected.html (added)
• LayoutTests/fast/table/anonymous-table-no-baseline-align.html (added)
• LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align-expected.html (added)
• LayoutTests/fast/table/inline-table-in-inline-block-last-baseline-align.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderTable.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderTable.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-08-09  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        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  <mikhail.pozdnyakov@intel.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-08-09  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        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  <abarth@webkit.org>
 

trunk/Source/WebCore/rendering/RenderTable.cpp

@@ -1207 +1207 @@
 }
 
-enum LineBox { FirstLineBox, LastLineBox };
-
-static LayoutUnit getLineBoxBaseline(const RenderTable* table, LineBox lineBox)
-{
-    if (table->isWritingModeRoot())
+LayoutUnit RenderTable::baselinePosition(FontBaseline baselineType, bool firstLine, LineDirectionMode direction, LinePositionMode linePositionMode) const
+{
+    LayoutUnit baseline = firstLineBoxBaseline();
+    if (baseline != -1)
+        return baseline;
+
+    return RenderBox::baselinePosition(baselineType, firstLine, direction, linePositionMode);
+}
+
+LayoutUnit RenderTable::lastLineBoxBaseline() const
+{
+    // 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();
-
-    const RenderTableSection* topNonEmptySection = table->topNonEmptySection();
+    recalcSectionsIfNeeded();
+
+    const RenderTableSection* topNonEmptySection = this->topNonEmptySection();
     if (!topNonEmptySection)
         return -1;
@@ -1224 +1241 @@
         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);
 }
 

trunk/Source/WebCore/rendering/RenderTable.h

@@ -252 +252 @@
     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;