Changeset 199156 in webkit

Timestamp:

Apr 7, 2016 7:55:54 AM (8 years ago)

Author:

Alan Bujtas

Message:

REGRESSION (197987): Ingredient lists on smittenkitchen.com are full justified instead of left justified.
​https://bugs.webkit.org/show_bug.cgi?id=156326
<rdar://problem/25519393>

Reviewed by Antti Koivisto.

According to the spec (​https://drafts.csswg.org/css-text-3/#text-align-property)
unless otherwise specified by text-align-last, the last line before
a forced break or the end of the block is start-aligned.

In this patch we check if a forced break is present and we apply text alignment accordingly.

Test: fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html

Source/WebCore:

• rendering/SimpleLineLayout.cpp:

(WebCore::SimpleLineLayout::LineState::lastFragment): Make it optional so that we don't just check against a default fragment.
(WebCore::SimpleLineLayout::createLineRuns):
(WebCore::SimpleLineLayout::justifyRuns): Do not compute first run index on the current line twice.
(WebCore::SimpleLineLayout::textAlignForLine):
(WebCore::SimpleLineLayout::closeLineEndingAndAdjustRuns):

LayoutTests:

• fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout-expected.html: Added.
• fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout-expected.html (added)
• LayoutTests/fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/SimpleLineLayout.cpp (modified) (6 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-04-07  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION (197987): Ingredient lists on smittenkitchen.com are full justified instead of left justified.
+        https://bugs.webkit.org/show_bug.cgi?id=156326
+        <rdar://problem/25519393>
+
+        Reviewed by Antti Koivisto.
+
+        According to the spec (https://drafts.csswg.org/css-text-3/#text-align-property) 
+        unless otherwise specified by text-align-last, the last line before
+        a forced break or the end of the block is start-aligned.
+
+        In this patch we check if a forced break is present and we apply text alignment accordingly.
+
+        Test: fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html
+
+        * fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout-expected.html: Added.
+        * fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html: Added.
+
 2016-04-07  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-04-07  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION (197987): Ingredient lists on smittenkitchen.com are full justified instead of left justified.
+        https://bugs.webkit.org/show_bug.cgi?id=156326
+        <rdar://problem/25519393>
+
+        Reviewed by Antti Koivisto.
+
+        According to the spec (https://drafts.csswg.org/css-text-3/#text-align-property) 
+        unless otherwise specified by text-align-last, the last line before
+        a forced break or the end of the block is start-aligned.
+
+        In this patch we check if a forced break is present and we apply text alignment accordingly.
+
+        Test: fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html
+
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::LineState::lastFragment): Make it optional so that we don't just check against a default fragment.
+        (WebCore::SimpleLineLayout::createLineRuns):
+        (WebCore::SimpleLineLayout::justifyRuns): Do not compute first run index on the current line twice.
+        (WebCore::SimpleLineLayout::textAlignForLine):
+        (WebCore::SimpleLineLayout::closeLineEndingAndAdjustRuns):
+
 2016-04-07  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/rendering/SimpleLineLayout.cpp

@@ -403 +403 @@
     const TextFragmentIterator::TextFragment& overflowedFragment() const { return m_overflowedFragment; }
     bool hasTrailingWhitespace() const { return m_trailingWhitespaceLength; }
-    TextFragmentIterator::TextFragment lastFragment() const { return m_fragments.last(); }
+    Optional<TextFragmentIterator::TextFragment> lastFragment() const
+    {
+        if (m_fragments.size())
+            return m_fragments.last();
+        return Nullopt;
+    }
     bool isWhitespaceOnly() const { return m_trailingWhitespaceWidth && m_runsWidth == m_trailingWhitespaceWidth; }
     bool fits(float extra) const { return m_availableWidth >= m_runsWidth + extra; }
@@ -734 +739 @@
             }
             // Non-breakable non-whitespace fragment when there's already content on the line. Push it to the next line.
-            if (line.lastFragment().overlapsToNextRenderer()) {
+            ASSERT(line.lastFragment());
+            if (line.lastFragment().value().overlapsToNextRenderer()) {
                 // Check if this fragment is a continuation of a previous segment. In such cases, we need to remove them all.
                 const auto& lastCompleteFragment = line.revertToLastCompleteFragment(runs);
@@ -758 +764 @@
 }
 
-static void justifyRuns(const LineState& line, Layout::RunVector& runs, Optional<unsigned> lastRunIndexOfPreviousLine)
+static void justifyRuns(const LineState& line, Layout::RunVector& runs, unsigned firstRunIndex)
 {
     ASSERT(runs.size());
@@ -765 +771 @@
         return;
 
-    auto firstRunIndex = lastRunIndexOfPreviousLine ? lastRunIndexOfPreviousLine.value() + 1 : 0;
     auto lastRunIndex = runs.size() - 1;
     ASSERT(firstRunIndex <= lastRunIndex);
@@ -795 +800 @@
 }
 
+static ETextAlign textAlignForLine(const TextFragmentIterator::Style& style, bool lastLine)
+{
+    // Fallback to LEFT (START) alignment for non-collapsable content and for the last line before a forced break or the end of the block.
+    auto textAlign = style.textAlign;
+    if (textAlign == JUSTIFY && (!style.collapseWhitespace || lastLine))
+        textAlign = LEFT;
+    return textAlign;
+}
+
 static void closeLineEndingAndAdjustRuns(LineState& line, Layout::RunVector& runs, Optional<unsigned> lastRunIndexOfPreviousLine, unsigned& lineCount,
-    const TextFragmentIterator& textFragmentIterator, bool lastLine)
+    const TextFragmentIterator& textFragmentIterator, bool lastLineInFlow)
 {
     if (!runs.size() || (lastRunIndexOfPreviousLine && runs.size() - 1 == lastRunIndexOfPreviousLine.value()))
@@ -805 +819 @@
     // Adjust runs' position by taking line's alignment into account.
     const auto& style = textFragmentIterator.style();
-    auto textAlign = style.textAlign;
-    // Fallback to LEFT alignment both for non-collapsable content and for the last line.
-    if (textAlign == JUSTIFY && (!style.collapseWhitespace || lastLine))
-        textAlign = LEFT;
-
+    auto firstRunIndex = lastRunIndexOfPreviousLine ? lastRunIndexOfPreviousLine.value() + 1 : 0;
     auto lineLogicalLeft = line.logicalLeftOffset();
+    auto textAlign = textAlignForLine(style, lastLineInFlow || (line.lastFragment() && line.lastFragment().value().type() == TextFragmentIterator::TextFragment::HardLineBreak));
     if (textAlign == JUSTIFY)
-        justifyRuns(line, runs, lastRunIndexOfPreviousLine);
+        justifyRuns(line, runs, firstRunIndex);
     else
         lineLogicalLeft = computeLineLeft(textAlign, line.availableWidth(), line.width(), line.logicalLeftOffset());
-    auto firstRunIndex = lastRunIndexOfPreviousLine ? lastRunIndexOfPreviousLine.value() + 1 : 0;
     for (auto i = firstRunIndex; i < runs.size(); ++i) {
         runs[i].logicalLeft += lineLogicalLeft;