Title: [216861] trunk
Revision
216861
Author
za...@apple.com
Date
2017-05-15 07:21:54 -0700 (Mon, 15 May 2017)

Log Message

Simple line layout: Leading whitespace followed by a <br> produces an extra linebreak.
https://bugs.webkit.org/show_bug.cgi?id=172076

Reviewed by Antti Koivisto.

Source/WebCore:

When the collapsed whitespace does not fit the line, we need to push it to the next line
so that we can decide whether any soft/hard linebreak should be skipped (to avoid double line breaks) or not.

Test: fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak.html

* rendering/SimpleLineLayout.cpp:
(WebCore::SimpleLineLayout::consumeLineBreakIfNeeded): special handling <br>
(WebCore::SimpleLineLayout::firstFragment): Now we need to deal with leading collapsed whitespace.
(WebCore::SimpleLineLayout::createLineRuns): We need to push even the collapsed whitespace to the next line.

LayoutTests:

* fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak-expected.html: Added.
* fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (216860 => 216861)


--- trunk/LayoutTests/ChangeLog	2017-05-15 09:14:25 UTC (rev 216860)
+++ trunk/LayoutTests/ChangeLog	2017-05-15 14:21:54 UTC (rev 216861)
@@ -1,3 +1,13 @@
+2017-05-15  Zalan Bujtas  <za...@apple.com>
+
+        Simple line layout: Leading whitespace followed by a <br> produces an extra linebreak.
+        https://bugs.webkit.org/show_bug.cgi?id=172076
+
+        Reviewed by Antti Koivisto.
+
+        * fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak-expected.html: Added.
+        * fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak.html: Added.
+
 2017-05-15  Nael Ouedraogo  <nael.ouedra...@crf.canon.fr>
 
         Invalid MediaSource duration value should throw TyperError instead of InvalidStateError

Added: trunk/LayoutTests/fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak-expected.html (0 => 216861)


--- trunk/LayoutTests/fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak-expected.html	2017-05-15 14:21:54 UTC (rev 216861)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that leading whitespace followed by soft/hard linebreaks.</title>
+<style>
+div {
+  width: 100px;
+  font-size: 13px;
+}
+</style>
+<script>
+if (window.internals)
+    internals.settings.setSimpleLineLayoutEnabled(false);
+</script>
+</head>
+<body>
+<div>foobarfoobar foobi <br>foobar</div>
+<div>foobarfoobar foobi 
+<br>foobar</div>
+<div>foobarfoobar foobi 
+ <br>foobar</div>
+</body>
+
+<div style="white-space: pre">
+<div>foobarfoobar foobi <br>foobar</div>
+<div>foobarfoobar foobi 
+<br>foobar</div>
+<div>foobarfoobar foobi 
+ <br>foobar</div>
+</div>
+<div style="white-space: pre-wrap">
+<div>foobarfoobar foobi <br>foobar</div>
+<div>foobarfoobar foobi 
+<br>foobar</div>
+<div>foobarfoobar foobi 
+ <br>foobar</div>
+</div>
+<div style="white-space: pre-line">
+<div>foobarfoobar foobi <br>foobar</div>
+<div>foobarfoobar foobi 
+<br>foobar</div>
+<div>foobarfoobar foobi 
+ <br>foobar</div>
+</div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak.html (0 => 216861)


--- trunk/LayoutTests/fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak.html	2017-05-15 14:21:54 UTC (rev 216861)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that leading whitespace followed by soft/hard linebreaks.</title>
+<style>
+div {
+  width: 100px;
+  font-size: 13px;
+}
+</style>
+</head>
+<body>
+<div>foobarfoobar foobi <br>foobar</div>
+<div>foobarfoobar foobi 
+<br>foobar</div>
+<div>foobarfoobar foobi 
+ <br>foobar</div>
+</body>
+
+<div style="white-space: pre">
+<div>foobarfoobar foobi <br>foobar</div>
+<div>foobarfoobar foobi 
+<br>foobar</div>
+<div>foobarfoobar foobi 
+ <br>foobar</div>
+</div>
+<div style="white-space: pre-wrap">
+<div>foobarfoobar foobi <br>foobar</div>
+<div>foobarfoobar foobi 
+<br>foobar</div>
+<div>foobarfoobar foobi 
+ <br>foobar</div>
+</div>
+<div style="white-space: pre-line">
+<div>foobarfoobar foobi <br>foobar</div>
+<div>foobarfoobar foobi 
+<br>foobar</div>
+<div>foobarfoobar foobi 
+ <br>foobar</div>
+</div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (216860 => 216861)


--- trunk/Source/WebCore/ChangeLog	2017-05-15 09:14:25 UTC (rev 216860)
+++ trunk/Source/WebCore/ChangeLog	2017-05-15 14:21:54 UTC (rev 216861)
@@ -1,3 +1,20 @@
+2017-05-15  Zalan Bujtas  <za...@apple.com>
+
+        Simple line layout: Leading whitespace followed by a <br> produces an extra linebreak.
+        https://bugs.webkit.org/show_bug.cgi?id=172076
+
+        Reviewed by Antti Koivisto.
+
+        When the collapsed whitespace does not fit the line, we need to push it to the next line
+        so that we can decide whether any soft/hard linebreak should be skipped (to avoid double line breaks) or not.
+
+        Test: fast/text/simple-line-layout-leading-whitespace-with-soft-hard-linebreak.html
+
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::consumeLineBreakIfNeeded): special handling <br>
+        (WebCore::SimpleLineLayout::firstFragment): Now we need to deal with leading collapsed whitespace.
+        (WebCore::SimpleLineLayout::createLineRuns): We need to push even the collapsed whitespace to the next line.
+
 2017-05-15  Nael Ouedraogo  <nael.ouedra...@crf.canon.fr>
 
         Invalid MediaSource duration value should throw TyperError instead of InvalidStateError

Modified: trunk/Source/WebCore/rendering/SimpleLineLayout.cpp (216860 => 216861)


--- trunk/Source/WebCore/rendering/SimpleLineLayout.cpp	2017-05-15 09:14:25 UTC (rev 216860)
+++ trunk/Source/WebCore/rendering/SimpleLineLayout.cpp	2017-05-15 14:21:54 UTC (rev 216861)
@@ -708,12 +708,16 @@
     if (!fragment.isLineBreak())
         return fragment;
 
-    if (preWrap(textFragmentIterator.style()) && preWrapLineBreakRule != PreWrapLineBreakRule::Ignore)
-        return fragment;
-
+    bool isHardLinebreak = fragment.type() == TextFragmentIterator::TextFragment::HardLineBreak;
     // <br> always produces a run. (required by testing output)
-    if (fragment.type() == TextFragmentIterator::TextFragment::HardLineBreak)
+    if (isHardLinebreak)
         line.appendFragmentAndCreateRunIfNeeded(fragment, runs);
+
+    auto& style = textFragmentIterator.style();
+    if (style.preserveNewline && preWrapLineBreakRule == PreWrapLineBreakRule::Preserve) {
+        if (!isHardLinebreak)
+            return fragment;
+    }
     return textFragmentIterator.nextTextFragment();
 }
 
@@ -730,18 +734,26 @@
 
 static TextFragmentIterator::TextFragment firstFragment(TextFragmentIterator& textFragmentIterator, LineState& currentLine, const LineState& previousLine, Layout::RunVector& runs)
 {
-    // Handle overflowed fragment from previous line.
-    TextFragmentIterator::TextFragment firstFragment(previousLine.overflowedFragment());
+    // Handle overflow fragment from previous line.
+    auto overflowedFragment = previousLine.overflowedFragment();
+    if (overflowedFragment.isEmpty())
+        return skipWhitespaceIfNeeded(textFragmentIterator.nextTextFragment(), textFragmentIterator);
 
-    if (firstFragment.isEmpty())
-        firstFragment = textFragmentIterator.nextTextFragment();
-    else if (firstFragment.type() == TextFragmentIterator::TextFragment::Whitespace && preWrap(textFragmentIterator.style()) && previousLine.firstCharacterFits()) {
-        // Special overflow pre-wrap whitespace handling: skip the overflowed whitespace (even when style says not-collapsible) if we managed to fit at least one character on the previous line.
-        firstFragment = textFragmentIterator.nextTextFragment();
+    if (overflowedFragment.type() != TextFragmentIterator::TextFragment::Whitespace)
+        return overflowedFragment;
+
+    // Leading whitespace handling.
+    auto& style = textFragmentIterator.style();
+    // Special overflow pre-wrap whitespace handling: skip the overflowed whitespace (even when style says not-collapsible)
+    // if we manage to fit at least one character on the previous line.
+    auto preWrapIsOn = preWrap(style);
+    if ((style.collapseWhitespace || preWrapIsOn) && previousLine.firstCharacterFits()) {
         // If skipping the whitespace puts us on a newline, skip the newline too as we already wrapped the line.
-        firstFragment = consumeLineBreakIfNeeded(firstFragment, textFragmentIterator, currentLine, runs, PreWrapLineBreakRule::Ignore);
+        auto firstFragmentCandidate = consumeLineBreakIfNeeded(textFragmentIterator.nextTextFragment(), textFragmentIterator, currentLine, runs,
+            preWrapIsOn ? PreWrapLineBreakRule::Ignore : PreWrapLineBreakRule::Preserve);
+        return skipWhitespaceIfNeeded(firstFragmentCandidate, textFragmentIterator);
     }
-    return skipWhitespaceIfNeeded(firstFragment, textFragmentIterator);
+    return skipWhitespaceIfNeeded(overflowedFragment, textFragmentIterator);
 }
 
 static void forceFragmentToLine(LineState& line, TextFragmentIterator& textFragmentIterator, Layout::RunVector& runs, const TextFragmentIterator::TextFragment& fragment)
@@ -773,7 +785,7 @@
     bool lineCanBeWrapped = style.wrapLines || style.breakFirstWordOnOverflow || style.breakAnyWordOnOverflow;
     auto fragment = firstFragment(textFragmentIterator, line, previousLine, runs);
     while (fragment.type() != TextFragmentIterator::TextFragment::ContentEnd) {
-        // Hard linebreak.
+        // Hard and soft linebreaks.
         if (fragment.isLineBreak()) {
             // Add the new line fragment only if there's nothing on the line. (otherwise the extra new line character would show up at the end of the content.)
             if (line.isEmpty() || fragment.type() == TextFragmentIterator::TextFragment::HardLineBreak) {
@@ -793,12 +805,14 @@
             bool emptyLine = line.isEmpty();
             // Whitespace fragment.
             if (fragment.type() == TextFragmentIterator::TextFragment::Whitespace) {
-                if (!style.collapseWhitespace) {
-                    // Split the fragment; (modified)fragment stays on this line, overflowedFragment is pushed to next line.
-                    line.setOverflowedFragment(splitFragmentToFitLine(fragment, line, textFragmentIterator));
-                    line.appendFragmentAndCreateRunIfNeeded(fragment, runs);
+                if (style.collapseWhitespace) {
+                    // Push collapased whitespace to the next line.
+                    line.setOverflowedFragment(fragment);
+                    break;
                 }
-                // When whitespace collapse is on, whitespace that doesn't fit is simply skipped.
+                // Split the whitespace; left part stays on this line, right is pushed to next line.
+                line.setOverflowedFragment(splitFragmentToFitLine(fragment, line, textFragmentIterator));
+                line.appendFragmentAndCreateRunIfNeeded(fragment, runs);
                 break;
             }
             // Non-whitespace fragment. (!style.wrapLines: bug138102(preserve existing behavior)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to