REGRESSION (197987): Ingredient lists on smittenkitchen.com are full justified instea...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Apr 2016 14:55:54 +0000 (14:55 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Apr 2016 14:55:54 +0000 (14:55 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout-expected.html [new file with mode: 0644]
LayoutTests/fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/SimpleLineLayout.cpp

index 9ab509b..b67830c 100644 (file)
@@ -1,3 +1,22 @@
+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>
 
         Shadow DOM: Implement display: contents for slots
diff --git a/LayoutTests/fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout-expected.html b/LayoutTests/fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout-expected.html
new file mode 100644 (file)
index 0000000..f33476d
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that ignore text-align: justify for last lines.</title>
+<style>
+div {
+    display: inline-block;
+    border: 1px solid green;
+    text-align: justify;
+    margin-left: 10px;
+}
+</style>
+</head>
+<body>
+<script>
+if (window.internals)
+    internals.settings.setSimpleLineLayoutEnabled(false);
+var width = 50;
+var content = "Lo rem ip<br>sum do lor<br>sit a met, con sec te tur<br>ad ip is cing el it<br>sed do e i us mod tempor.";
+for (var i = 0; i < 20; ++i, width += 5) {
+    var element = document.createElement("div");
+    element.style.width = width + "px";
+    element.innerHTML = content;
+    document.body.appendChild(element);
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html b/LayoutTests/fast/css3-text/css3-text-justify/text-justify-last-line-simple-line-layout.html
new file mode 100644 (file)
index 0000000..0379ceb
--- /dev/null
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that ignore text-align: justify for last lines.</title>
+<style>
+div {
+    display: inline-block;
+    border: 1px solid green;
+    text-align: justify;
+    margin-left: 10px;
+}
+</style>
+</head>
+<body>
+<script>
+var width = 50;
+var content = "Lo rem ip<br>sum do lor<br>sit a met, con sec te tur<br>ad ip is cing el it<br>sed do e i us mod tempor.";
+for (var i = 0; i < 20; ++i, width += 5) {
+    var element = document.createElement("div");
+    element.style.width = width + "px";
+    element.innerHTML = content;
+    document.body.appendChild(element);
+}
+</script>
+</body>
+</html>
index 39694a5..55205e8 100644 (file)
@@ -1,3 +1,26 @@
+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>
 
         FrameView::qualifiesAsVisuallyNonEmpty() returns false when loading a Google search results page before search results are loaded, even though the header is visible
index 7d6bc8e..ea5ed53 100644 (file)
@@ -402,7 +402,12 @@ public:
     float logicalLeftOffset() const { return m_logicalLeftOffset; }
     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; }
     bool firstCharacterFits() const { return m_firstCharacterFits; }
@@ -733,7 +738,8 @@ static bool createLineRuns(LineState& line, const LineState& previousLine, Layou
                 break;
             }
             // 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);
                 textFragmentIterator.revertToEndOfFragment(lastCompleteFragment);
@@ -757,14 +763,13 @@ static ExpansionBehavior expansionBehavior(bool isAfterExpansion, bool lastRunOn
     return expansionBehavior;
 }
 
-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());
     auto widthToDistribute = line.availableWidth() - line.width();
     if (widthToDistribute <= 0)
         return;
 
-    auto firstRunIndex = lastRunIndexOfPreviousLine ? lastRunIndexOfPreviousLine.value() + 1 : 0;
     auto lastRunIndex = runs.size() - 1;
     ASSERT(firstRunIndex <= lastRunIndex);
     Vector<std::pair<unsigned, ExpansionBehavior>> expansionOpportunityList;
@@ -794,8 +799,17 @@ static void justifyRuns(const LineState& line, Layout::RunVector& runs, Optional
     }
 }
 
+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()))
         return;
@@ -804,17 +818,13 @@ static void closeLineEndingAndAdjustRuns(LineState& line, Layout::RunVector& run
         return;
     // 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;
         runs[i].logicalRight += lineLogicalLeft;