Changeset 108111 in webkit

Timestamp:

Feb 17, 2012 1:10:42 PM (12 years ago)

Author:

robert@webkit.org

Message:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/empty-span-expected.html (added)
• LayoutTests/fast/css/empty-span.html (added)
• LayoutTests/fast/css/non-empty-span.html (added)
• LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.png (added)
• LayoutTests/platform/chromium-linux-x86/fast/css/non-empty-span-expected.txt (added)
• LayoutTests/platform/chromium/test_expectations.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockLineLayout.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-01-23  Robert Hogan  <robert@webkit.org>
+
+        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  <inferno@chromium.org>
 

trunk/LayoutTests/platform/chromium/test_expectations.txt

@@ -4207 +4207 @@
 // 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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-01-23  Robert Hogan  <robert@webkit.org>
+
+        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  <rtoy@google.com>
 

trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

@@ -491 +491 @@
         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);
@@ -1832 +1835 @@
 }
 
-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();
 }
@@ -1848 +1859 @@
         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;
 
@@ -2236 +2247 @@
             // 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();