REGRESSION: Inline-block baseline is wrong when zero-width replaced child is present
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Sep 2015 14:42:44 +0000 (14:42 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Sep 2015 14:42:44 +0000 (14:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=147452
rdar://problem/21943074

Reviewed by Myles Maxfield.

Source/WebCore:

Added new test in fast/inline-block

Treat zero width replaced elements the same as replaced elements with width. Instead of
clearing floats based off having no committed width, we instead track both committed
width and committed replaced objects. We do this with two new booleans in LineWidth
so that we know when we have uncomitted and committed replaced objects.

* rendering/line/BreakingContext.h:
(WebCore::BreakingContext::handleReplaced):
(WebCore::BreakingContext::handleText):
(WebCore::BreakingContext::canBreakAtThisPosition):
(WebCore::BreakingContext::commitAndUpdateLineBreakIfNeeded):
* rendering/line/LineWidth.cpp:
(WebCore::LineWidth::LineWidth):
(WebCore::LineWidth::commit):
(WebCore::LineWidth::applyOverhang):
* rendering/line/LineWidth.h:
(WebCore::LineWidth::committedWidth):
(WebCore::LineWidth::availableWidth):
(WebCore::LineWidth::logicalLeftOffset):
(WebCore::LineWidth::hasCommitted):
(WebCore::LineWidth::addUncommittedWidth):
(WebCore::LineWidth::addUncommittedReplacedWidth):

LayoutTests:

* fast/inline-block/baseline-with-zero-width-replaced-child-expected.html: Added.
* fast/inline-block/baseline-with-zero-width-replaced-child.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@189540 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/inline-block/baseline-with-zero-width-replaced-child-expected.html [new file with mode: 0644]
LayoutTests/fast/inline-block/baseline-with-zero-width-replaced-child.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/line/BreakingContext.h
Source/WebCore/rendering/line/LineWidth.cpp
Source/WebCore/rendering/line/LineWidth.h

index ec2778c..64168ee 100644 (file)
@@ -1,3 +1,14 @@
+2015-09-09  David Hyatt  <hyatt@apple.com>
+
+        REGRESSION: Inline-block baseline is wrong when zero-width replaced child is present
+        https://bugs.webkit.org/show_bug.cgi?id=147452
+        rdar://problem/21943074
+
+        Reviewed by Myles Maxfield.
+
+        * fast/inline-block/baseline-with-zero-width-replaced-child-expected.html: Added.
+        * fast/inline-block/baseline-with-zero-width-replaced-child.html: Added.
+
 2015-09-09  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Percentage columns shouldn't include border and padding
diff --git a/LayoutTests/fast/inline-block/baseline-with-zero-width-replaced-child-expected.html b/LayoutTests/fast/inline-block/baseline-with-zero-width-replaced-child-expected.html
new file mode 100644 (file)
index 0000000..d6eb39c
--- /dev/null
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+The green block should align nicely with the text.
+<div style="display: inline-block; height: 20px; background-color: green;"><div style="width: 45px; display:inline-block;height:3px;"></div>
+</body>
+</html>
diff --git a/LayoutTests/fast/inline-block/baseline-with-zero-width-replaced-child.html b/LayoutTests/fast/inline-block/baseline-with-zero-width-replaced-child.html
new file mode 100644 (file)
index 0000000..2885bdc
--- /dev/null
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+The green block should align nicely with the text.
+<div style="display: inline-block; height: 20px; background-color: green;"><div style="display: inline-block; background-color: red;"></div><div style="height: 20px; width: 45px; float: right;"></div>
+</body>
+</html>
index 6f06517..2fa5e1f 100644 (file)
@@ -1,3 +1,35 @@
+2015-09-08  David Hyatt  <hyatt@apple.com>
+
+        REGRESSION: Inline-block baseline is wrong when zero-width replaced child is present
+        https://bugs.webkit.org/show_bug.cgi?id=147452
+        rdar://problem/21943074
+
+        Reviewed by Myles Maxfield.
+
+        Added new test in fast/inline-block
+
+        Treat zero width replaced elements the same as replaced elements with width. Instead of
+        clearing floats based off having no committed width, we instead track both committed
+        width and committed replaced objects. We do this with two new booleans in LineWidth
+        so that we know when we have uncomitted and committed replaced objects.
+
+        * rendering/line/BreakingContext.h:
+        (WebCore::BreakingContext::handleReplaced):
+        (WebCore::BreakingContext::handleText):
+        (WebCore::BreakingContext::canBreakAtThisPosition):
+        (WebCore::BreakingContext::commitAndUpdateLineBreakIfNeeded):
+        * rendering/line/LineWidth.cpp:
+        (WebCore::LineWidth::LineWidth):
+        (WebCore::LineWidth::commit):
+        (WebCore::LineWidth::applyOverhang):
+        * rendering/line/LineWidth.h:
+        (WebCore::LineWidth::committedWidth):
+        (WebCore::LineWidth::availableWidth):
+        (WebCore::LineWidth::logicalLeftOffset):
+        (WebCore::LineWidth::hasCommitted):
+        (WebCore::LineWidth::addUncommittedWidth):
+        (WebCore::LineWidth::addUncommittedReplacedWidth):
+
 2015-09-09  Antti Koivisto  <antti@apple.com>
 
         Split mixed font GlyphPage functionality to separate class
index 802ee43..41fc3f0 100644 (file)
@@ -552,9 +552,9 @@ inline void BreakingContext::handleReplaced()
             m_ignoringSpaces = true;
         }
         if (downcast<RenderListMarker>(*m_current.renderer()).isInside())
-            m_width.addUncommittedWidth(replacedLogicalWidth);
+            m_width.addUncommittedReplacedWidth(replacedLogicalWidth);
     } else
-        m_width.addUncommittedWidth(replacedLogicalWidth);
+        m_width.addUncommittedReplacedWidth(replacedLogicalWidth);
     if (is<RenderRubyRun>(*m_current.renderer())) {
         m_width.applyOverhang(downcast<RenderRubyRun>(m_current.renderer()), m_lastObject, m_nextObject);
         downcast<RenderRubyRun>(m_current.renderer())->updatePriorContextFromCachedBreakIterator(m_renderTextInfo.lineBreakIterator);
@@ -741,7 +741,7 @@ inline bool BreakingContext::handleText(WordMeasurements& wordMeasurements, bool
     bool breakNBSP = m_autoWrap && m_currentStyle->nbspMode() == SPACE;
     // Auto-wrapping text should wrap in the middle of a word only if it could not wrap before the word,
     // which is only possible if the word is the first thing on the line, that is, if |w| is zero.
-    bool breakWords = m_currentStyle->breakWords() && ((m_autoWrap && !m_width.committedWidth()) || m_currWS == PRE);
+    bool breakWords = m_currentStyle->breakWords() && ((m_autoWrap && !m_width.hasCommitted()) || m_currWS == PRE);
     bool midWordBreak = false;
     bool breakAll = m_currentStyle->wordBreak() == BreakAllWordBreak && m_autoWrap;
     bool keepAllWords = m_currentStyle->wordBreak() == KeepAllWordBreak;
@@ -858,7 +858,7 @@ inline bool BreakingContext::handleText(WordMeasurements& wordMeasurements, bool
 
             applyWordSpacing = wordSpacing && m_currentCharacterIsSpace;
 
-            if (!m_width.committedWidth() && m_autoWrap && !m_width.fitsOnLine())
+            if (!m_width.hasCommitted() && m_autoWrap && !m_width.fitsOnLine())
                 m_width.fitBelowFloats(m_lineInfo.isFirstLine());
 
             if (m_autoWrap || breakWords) {
@@ -1086,7 +1086,7 @@ inline bool BreakingContext::canBreakAtThisPosition()
     bool canBreakHere = !m_currentCharacterIsSpace && textBeginsWithBreakablePosition(nextRenderText);
 
     // See if attempting to fit below floats creates more available width on the line.
-    if (!m_width.fitsOnLine() && !m_width.committedWidth())
+    if (!m_width.fitsOnLine() && !m_width.hasCommitted())
         m_width.fitBelowFloats(m_lineInfo.isFirstLine());
 
     bool canPlaceOnLine = m_width.fitsOnLine() || !m_autoWrapWasEverTrueOnLine;
@@ -1120,7 +1120,7 @@ inline void BreakingContext::commitAndUpdateLineBreakIfNeeded()
             m_atEnd = true;
             return;
         }
-    } else if (m_blockStyle.autoWrap() && !m_width.fitsOnLine() && !m_width.committedWidth()) {
+    } else if (m_blockStyle.autoWrap() && !m_width.fitsOnLine() && !m_width.hasCommitted()) {
         // If the container autowraps but the current child does not then we still need to ensure that it
         // wraps and moves below any floats.
         m_width.fitBelowFloats(m_lineInfo.isFirstLine());
index 0b17d18..796e005 100644 (file)
@@ -125,6 +125,10 @@ void LineWidth::commit()
 {
     m_committedWidth += m_uncommittedWidth;
     m_uncommittedWidth = 0;
+    if (m_hasUncommittedReplaced) {
+        m_hasCommittedReplaced = true;
+        m_hasUncommittedReplaced = false;
+    }
 }
 
 void LineWidth::applyOverhang(RenderRubyRun* rubyRun, RenderObject* startRenderer, RenderObject* endRenderer)
index d00f3a5..e744b1f 100644 (file)
@@ -58,10 +58,20 @@ public:
     float committedWidth() const { return m_committedWidth; }
     float availableWidth() const { return m_availableWidth; }
     float logicalLeftOffset() const { return m_left; }
+    
+    bool hasCommitted() const { return m_committedWidth > 0 || m_hasCommittedReplaced; }
 
     void updateAvailableWidth(LayoutUnit minimumHeight = 0);
     void shrinkAvailableWidthForNewFloatIfNeeded(FloatingObject*);
-    void addUncommittedWidth(float delta) { m_uncommittedWidth += delta; }
+    void addUncommittedWidth(float delta)
+    {
+        m_uncommittedWidth += delta;
+    }
+    void addUncommittedReplacedWidth(float delta)
+    {
+        addUncommittedWidth(delta);
+        m_hasUncommittedReplaced = true;
+    }
     void commit();
     void applyOverhang(RenderRubyRun*, RenderObject* startRenderer, RenderObject* endRenderer);
     void fitBelowFloats(bool isFirstLine = false);
@@ -86,6 +96,8 @@ private:
     float m_right;
     float m_availableWidth;
     bool m_isFirstLine;
+    bool m_hasUncommittedReplaced { false };
+    bool m_hasCommittedReplaced { false };
     IndentTextOrNot m_shouldIndentText;
 };