- Revision
- 109047
- Author
- rn...@webkit.org
- Date
- 2012-02-27 17:19:47 -0800 (Mon, 27 Feb 2012)
Log Message
Merge 108111 - REGRESSION: empty span creates renders with non-zero height
https://bugs.webkit.org/show_bug.cgi?id=76465
Reviewed by David Hyatt.
Source/WebCore:
Tests: fast/css/empty-span.html
fast/css/non-empty-span.html
Empty inlines with line-height, vertical-alignment or font metrics should only get a linebox if there is some
other content in the line. So only create line boxes for such elements on lines that are not empty.
This patch fixes a regression where an empty inline with line-height was propagating its height to an empty line.
It also fixes cases where lines with content that had a leading empty inline element weren't respecting the
vertical alignment or font-height of the empty inline.
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlock::constructLine): only create line boxes for lines that are not empty.
(WebCore::requiresLineBoxForContent): an inline flow with line-height, vertical-alignment, or font-size
will need a linebox if the rest of the line is not empty.
(WebCore):
(WebCore::alwaysRequiresLineBox): rename from inlineFlowRequiresLineBox.
(WebCore::requiresLineBox):
(WebCore::RenderBlock::LineBreaker::nextLineBreak): if the inline flow definitely requires a line, mark
the line non-empty - otherwise hold off.
LayoutTests:
* fast/css/empty-span-expected.html: Added.
* fast/css/empty-span.html: Added.
* fast/css/non-empty-span.html: Added.
* platform/chromium/test_expectations.txt: Suppress result until rebaseline on MAC and WIN.
* platform/chromium-linux-x86/fast/css/non-empty-span-expected.png: Added.
* platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt: Added.
TBR=rob...@webkit.org
Review URL: https://chromiumcodereview.appspot.com/9478022
Modified Paths
Added Paths
Diff
Copied: branches/chromium/1025/LayoutTests/fast/css/empty-span-expected.html (from rev 108111, trunk/LayoutTests/fast/css/empty-span-expected.html) (0 => 109047)
--- branches/chromium/1025/LayoutTests/fast/css/empty-span-expected.html (rev 0)
+++ branches/chromium/1025/LayoutTests/fast/css/empty-span-expected.html 2012-02-28 01:19:47 UTC (rev 109047)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+ <head></head>
+ <body>
+ <div>Before empty span</div>
+ <span></span>
+ <div>After empty span</div>
+ <div>Before empty span</div>
+ <span></span>
+ <div>After empty span</div>
+ <div>Before empty span</div>
+ <span></span>
+ <div>After empty span</div>
+ </body>
+</html>
\ No newline at end of file
Copied: branches/chromium/1025/LayoutTests/fast/css/empty-span.html (from rev 108111, trunk/LayoutTests/fast/css/empty-span.html) (0 => 109047)
--- branches/chromium/1025/LayoutTests/fast/css/empty-span.html (rev 0)
+++ branches/chromium/1025/LayoutTests/fast/css/empty-span.html 2012-02-28 01:19:47 UTC (rev 109047)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+ <head></head>
+ <body>
+<!-- https://bugs.webkit.org/show_bug.cgi?id=76465
+ There should be no extra space between any of the lines. -->
+ <div>Before empty span</div>
+ <span style="line-height:5.24;"></span>
+ <div>After empty span</div>
+ <div>Before empty span</div>
+ <span style="font-size:100px;"></span>
+ <div>After empty span</div>
+ <div>Before empty span</div>
+ <span style="vertical-align:100px;"></span>
+ <div>After empty span</div>
+ </body>
+</html>
\ No newline at end of file
Copied: branches/chromium/1025/LayoutTests/fast/css/non-empty-span.html (from rev 108111, trunk/LayoutTests/fast/css/non-empty-span.html) (0 => 109047)
--- branches/chromium/1025/LayoutTests/fast/css/non-empty-span.html (rev 0)
+++ branches/chromium/1025/LayoutTests/fast/css/non-empty-span.html 2012-02-28 01:19:47 UTC (rev 109047)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+ <head></head>
+ <body>
+<!-- https://bugs.webkit.org/show_bug.cgi?id=76465
+ There should be 100px space between the 'before' and 'after' lines (disregarding the 'X'). -->
+ <div>Before empty span</div>
+ <span style="line-height:100px;"></span>X
+ <div>After empty span</div>
+ <div>Before empty span</div>
+ <span style="font-size:100px;"></span>X
+ <div>After empty span</div>
+ <div>Before empty span</div>
+ <span style="vertical-align:100px;"></span>X
+ <div>After empty span</div>
+ </body>
+</html>
\ No newline at end of file
Copied: branches/chromium/1025/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.png (from rev 108111, trunk/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.png)
(Binary files differ)
Copied: branches/chromium/1025/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt (from rev 108111, trunk/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt) (0 => 109047)
--- branches/chromium/1025/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt (rev 0)
+++ branches/chromium/1025/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt 2012-02-28 01:19:47 UTC (rev 109047)
@@ -0,0 +1,35 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x474
+ RenderBlock {HTML} at (0,0) size 800x474
+ RenderBody {BODY} at (8,8) size 784x458
+ RenderBlock {DIV} at (0,0) size 784x20
+ RenderText {#text} at (0,0) size 114x19
+ text run at (0,0) width 114: "Before empty span"
+ RenderBlock (anonymous) at (0,20) size 784x100
+ RenderInline {SPAN} at (0,0) size 0x19
+ RenderText {#text} at (0,40) size 11x19
+ text run at (0,40) width 11: "X"
+ RenderBlock {DIV} at (0,120) size 784x20
+ RenderText {#text} at (0,0) size 104x19
+ text run at (0,0) width 104: "After empty span"
+ RenderBlock {DIV} at (0,140) size 784x20
+ RenderText {#text} at (0,0) size 114x19
+ text run at (0,0) width 114: "Before empty span"
+ RenderBlock (anonymous) at (0,160) size 784x118
+ RenderInline {SPAN} at (0,0) size 0x114
+ RenderText {#text} at (0,77) size 11x19
+ text run at (0,77) width 11: "X"
+ RenderBlock {DIV} at (0,278) size 784x20
+ RenderText {#text} at (0,0) size 104x19
+ text run at (0,0) width 104: "After empty span"
+ RenderBlock {DIV} at (0,298) size 784x20
+ RenderText {#text} at (0,0) size 114x19
+ text run at (0,0) width 114: "Before empty span"
+ RenderBlock (anonymous) at (0,318) size 784x120
+ RenderInline {SPAN} at (0,0) size 0x19
+ RenderText {#text} at (0,100) size 11x19
+ text run at (0,100) width 11: "X"
+ RenderBlock {DIV} at (0,438) size 784x20
+ RenderText {#text} at (0,0) size 104x19
+ text run at (0,0) width 104: "After empty span"
Modified: branches/chromium/1025/Source/WebCore/rendering/RenderBlockLineLayout.cpp (109046 => 109047)
--- branches/chromium/1025/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2012-02-28 01:16:55 UTC (rev 109046)
+++ branches/chromium/1025/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2012-02-28 01:19:47 UTC (rev 109047)
@@ -491,6 +491,9 @@
if (runCount == 2 && !r->m_object->isListMarker())
isOnlyRun = (!style()->isLeftToRightDirection() ? bidiRuns.lastRun() : bidiRuns.firstRun())->m_object->isListMarker();
+ if (lineInfo.isEmpty())
+ continue;
+
InlineBox* box = createInlineBoxForRenderer(r->m_object, false, isOnlyRun);
r->m_box = box;
@@ -1824,14 +1827,22 @@
|| (whitespacePosition == TrailingWhitespace && style->whiteSpace() == PRE_WRAP && (!lineInfo.isEmpty() || !lineInfo.previousLineBrokeCleanly()));
}
-static bool inlineFlowRequiresLineBox(RenderInline* flow, const LineInfo& lineInfo)
+static bool requiresLineBoxForContent(RenderInline* flow, const LineInfo& lineInfo)
{
+ RenderObject* parent = flow->parent();
+ if (flow->document()->inNoQuirksMode()
+ && (flow->style(lineInfo.isFirstLine())->lineHeight() != parent->style(lineInfo.isFirstLine())->lineHeight()
+ || flow->style()->verticalAlign() != parent->style()->verticalAlign()
+ || !parent->style()->font().fontMetrics().hasIdenticalAscentDescentAndLineGap(flow->style()->font().fontMetrics())))
+ return true;
+ return false;
+}
+
+static bool alwaysRequiresLineBox(RenderInline* flow, const LineInfo& lineInfo)
+{
// FIXME: Right now, we only allow line boxes for inlines that are truly empty.
// We need to fix this, though, because at the very least, inlines containing only
// ignorable whitespace should should also have line boxes.
- if (!flow->document()->inQuirksMode() && flow->style(lineInfo.isFirstLine())->lineHeight() != flow->parent()->style(lineInfo.isFirstLine())->lineHeight())
- return true;
-
return !flow->firstChild() && flow->hasInlineDirectionBordersPaddingOrMargin();
}
@@ -1840,7 +1851,7 @@
if (it.m_obj->isFloatingOrPositioned())
return false;
- if (it.m_obj->isRenderInline() && !inlineFlowRequiresLineBox(toRenderInline(it.m_obj), lineInfo))
+ if (it.m_obj->isRenderInline() && !alwaysRequiresLineBox(toRenderInline(it.m_obj), lineInfo) && !requiresLineBoxForContent(toRenderInline(it.m_obj), lineInfo))
return false;
if (!shouldCollapseWhiteSpace(it.m_obj->style(), lineInfo, whitespacePosition) || it.m_obj->isBR())
@@ -2228,8 +2239,12 @@
// to make sure that we stop to include this object and then start ignoring spaces again.
// If this object is at the start of the line, we need to behave like list markers and
// start ignoring spaces.
- if (inlineFlowRequiresLineBox(flowBox, lineInfo)) {
- lineInfo.setEmpty(false, m_block, &width);
+ bool requiresLineBox = alwaysRequiresLineBox(flowBox, lineInfo);
+ if (requiresLineBox || requiresLineBoxForContent(flowBox, lineInfo)) {
+ // An empty inline that only has line-height, vertical-align or font-metrics will only get a
+ // line box to affect the height of the line if the rest of the line is not empty.
+ if (requiresLineBox)
+ lineInfo.setEmpty(false, m_block, &width);
if (ignoringSpaces) {
trailingObjects.clear();
addMidpoint(lineMidpointState, InlineIterator(0, current.m_obj, 0)); // Stop ignoring spaces.
