Changeset 102538 in webkit

Timestamp:

Dec 11, 2011, 11:06:56 AM (13 years ago)

Author:

mitz@apple.com

Message:

Source/WebCore: <rdar://problem/10561285> REGRESSION (r80438): First word on a line or after collapsed space may not be hyphenated even though it should
​https://bugs.webkit.org/show_bug.cgi?id=74239

Reviewed by Anders Carlsson.

Tests: fast/text/hyphenate-first-word-after-skipped-space-expected.html

fast/text/hyphenate-first-word-after-skipped-space.html

• rendering/RenderBlockLineLayout.cpp:

(WebCore::tryHyphenating): Replaced the assumption that the character at lastSpace is a space
iff lastSpace is non-zero with a test of whether it is a space, in the sense that it should
not be counted as part of the prefix when comparing it to the value of hyphenate-limit-before.

LayoutTests: Test and updated results for <rdar://problem/10561285> REGRESSION (r80438): First word on a line or after collapsed space may not be hyphenated even though it should
​https://bugs.webkit.org/show_bug.cgi?id=74239

Reviewed by Anders Carlsson.

• fast/text/hyphenate-first-word-after-skipped-space-expected.html: Added.
• fast/text/hyphenate-first-word-after-skipped-space.html: Added.
• platform/mac/fast/text/hyphenate-limit-lines-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/hyphenate-first-word-after-skipped-space-expected.html (added)
• LayoutTests/fast/text/hyphenate-first-word-after-skipped-space.html (added)
• LayoutTests/platform/mac/fast/text/hyphenate-limit-lines-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockLineLayout.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog ¶

@@ +1 @@
+2011-12-11  Dan Bernstein  <mitz@apple.com>
+
+        Test and updated results for <rdar://problem/10561285> REGRESSION (r80438): First word on a line or after collapsed space may not be hyphenated even though it should
+        https://bugs.webkit.org/show_bug.cgi?id=74239
+
+        Reviewed by Anders Carlsson.
+
+        * fast/text/hyphenate-first-word-after-skipped-space-expected.html: Added.
+        * fast/text/hyphenate-first-word-after-skipped-space.html: Added.
+        * platform/mac/fast/text/hyphenate-limit-lines-expected.txt:
+
 2011-12-09  Robert Hogan  <robert@webkit.org>
 

trunk/LayoutTests/platform/mac/fast/text/hyphenate-limit-lines-expected.txt ¶

@@ -79 +79 @@
           text run at (1,156) width 200: "pleasures and de" + hyphen string "-"
           text run at (1,187) width 200: "sires into"
-          text run at (1,218) width 200: "necessary and"
-          text run at (1,249) width 155: "unnecessary\x{2014}"
-          text run at (1,280) width 200: "these and other"
-          text run at (1,311) width 200: "great forms of"
-          text run at (1,342) width 200: "thought are all of"
-          text run at (1,373) width 66: "them "
-          text run at (67,373) width 134: "to be found"
-          text run at (1,404) width 200: "in the Republic,"
-          text run at (1,435) width 200: "and were probably"
+          text run at (1,218) width 165: "necessary and "
+          text run at (166,218) width 35: "un" + hyphen string "-"
+          text run at (1,249) width 183: "necessary\x{2014}these"
+          text run at (1,280) width 200: "and other great"
+          text run at (1,311) width 200: "forms of thought"
+          text run at (1,342) width 180: "are all of them "
+          text run at (181,342) width 20: "to"
+          text run at (1,373) width 200: "be found in the"
+          text run at (1,404) width 200: "Republic, and"
+          text run at (1,435) width 200: "were probably"
           text run at (1,466) width 200: "first invented by"
           text run at (1,497) width 60: "Plato."

trunk/Source/WebCore/ChangeLog ¶

@@ +1 @@
+2011-12-11  Dan Bernstein  <mitz@apple.com>
+
+        <rdar://problem/10561285> REGRESSION (r80438): First word on a line or after collapsed space may not be hyphenated even though it should
+        https://bugs.webkit.org/show_bug.cgi?id=74239
+
+        Reviewed by Anders Carlsson.
+
+        Tests: fast/text/hyphenate-first-word-after-skipped-space-expected.html
+               fast/text/hyphenate-first-word-after-skipped-space.html
+
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::tryHyphenating): Replaced the assumption that the character at lastSpace is a space
+        iff lastSpace is non-zero with a test of whether it is a space, in the sense that it should
+        not be counted as part of the prefix when comparing it to the value of hyphenate-limit-before.
+
 2011-12-10  Benjamin Poulain  <bpoulain@apple.com>
 

trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp ¶

@@ -1941 +1941 @@
 
     prefixLength = lastHyphenLocation(text->characters() + lastSpace, pos - lastSpace, min(prefixLength, static_cast<unsigned>(pos - lastSpace - minimumSuffixLength)) + 1, localeIdentifier);
-    // FIXME: The following assumes that the character at lastSpace is a space (and therefore should not factor
-    // into hyphenate-limit-before) unless lastSpace is 0. This is wrong in the rare case of hyphenating
-    // the first word in a text node which has leading whitespace.
-    if (!prefixLength || prefixLength - (lastSpace ? 1 : 0) < static_cast<unsigned>(minimumPrefixLength))
+    if (!prefixLength || prefixLength < static_cast<unsigned>(minimumPrefixLength))
         return;
+
+    // When lastSapce is a space, which it always is except sometimes at the beginning of a line or after collapsed
+    // space, it should not count towards hyphenate-limit-before.
+    if (prefixLength == static_cast<unsigned>(minimumPrefixLength)) {
+        UChar characterAtLastSpace = text->characters()[lastSpace];
+        if (characterAtLastSpace == ' ' || characterAtLastSpace == '\n' || characterAtLastSpace == '\t' || characterAtLastSpace == noBreakSpace)
+            return;
+    }
 
     ASSERT(pos - lastSpace - prefixLength >= static_cast<unsigned>(minimumSuffixLength));