Diff
Modified: trunk/LayoutTests/ChangeLog (108110 => 108111)
--- trunk/LayoutTests/ChangeLog 2012-02-17 20:57:02 UTC (rev 108110)
+++ trunk/LayoutTests/ChangeLog 2012-02-17 21:10:42 UTC (rev 108111)
@@ -1,3 +1,17 @@
+2012-01-23 Robert Hogan <[email protected]>
+
+ REGRESSION: empty span creates renders with non-zero height
+ https://bugs.webkit.org/show_bug.cgi?id=76465
+
+ Reviewed by David Hyatt.
+
+ * 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.
+
2012-02-17 Abhishek Arya <[email protected]>
:before content incorrectly placed in continuation
Added: trunk/LayoutTests/fast/css/empty-span-expected.html (0 => 108111)
--- trunk/LayoutTests/fast/css/empty-span-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css/empty-span-expected.html 2012-02-17 21:10:42 UTC (rev 108111)
@@ -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
Property changes on: trunk/LayoutTests/fast/css/empty-span-expected.html
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/css/empty-span.html (0 => 108111)
--- trunk/LayoutTests/fast/css/empty-span.html (rev 0)
+++ trunk/LayoutTests/fast/css/empty-span.html 2012-02-17 21:10:42 UTC (rev 108111)
@@ -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
Property changes on: trunk/LayoutTests/fast/css/empty-span.html
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/css/non-empty-span.html (0 => 108111)
--- trunk/LayoutTests/fast/css/non-empty-span.html (rev 0)
+++ trunk/LayoutTests/fast/css/non-empty-span.html 2012-02-17 21:10:42 UTC (rev 108111)
@@ -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
Property changes on: trunk/LayoutTests/fast/css/non-empty-span.html
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (108110 => 108111)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2012-02-17 20:57:02 UTC (rev 108110)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2012-02-17 21:10:42 UTC (rev 108111)
@@ -4206,3 +4206,6 @@
// Started failing http://trac.webkit.org/log/?verbose=on&rev=107837&stop_rev=107836
BUGWK78755 : compositing/culling/scrolled-within-boxshadow.html = IMAGE
+
+// Needs baselines for MAC and WIN.
+BUGWK76465 MAC WIN : fast/css/non-empty-span.html = IMAGE+TEXT
Added: trunk/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.png
(Binary files differ)
Added: svn:mime-type
Added: trunk/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt (0 => 108111)
--- trunk/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt 2012-02-17 21:10:42 UTC (rev 108111)
@@ -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"
Property changes on: trunk/LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (108110 => 108111)
--- trunk/Source/WebCore/ChangeLog 2012-02-17 20:57:02 UTC (rev 108110)
+++ trunk/Source/WebCore/ChangeLog 2012-02-17 21:10:42 UTC (rev 108111)
@@ -1,3 +1,30 @@
+2012-01-23 Robert Hogan <[email protected]>
+
+ REGRESSION: empty span creates renders with non-zero height
+ https://bugs.webkit.org/show_bug.cgi?id=76465
+
+ Reviewed by David Hyatt.
+
+ 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.
+
2012-02-17 Raymond Toy <[email protected]>
RealtimeAnalyserNode does not consistently respect .minDecibels
Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (108110 => 108111)
--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2012-02-17 20:57:02 UTC (rev 108110)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2012-02-17 21:10:42 UTC (rev 108111)
@@ -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;
@@ -1831,14 +1834,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();
}
@@ -1847,7 +1858,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())
@@ -2235,8 +2246,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.
