Bug 1293584 - Fix implementation of word-break:keep-all to better follow the spec. r=m_kato
authorJonathan Kew <jkew@mozilla.com>
Tue, 21 Jan 2020 08:02:20 +0000
changeset 510901 cf48dfafa73c1b7e9dd59cc8c70764e36ab06c56
parent 510900 23d86664257e5cd0b122b7e1a16aad181256f058
child 510902 f2b46092eeb876758677e8ffa2043f358a2edb1c
push id37041
push usershindli@mozilla.com
push dateTue, 21 Jan 2020 16:14:08 +0000
treeherdermozilla-central@875ae8e3ce6f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1293584
milestone74.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1293584 - Fix implementation of word-break:keep-all to better follow the spec. r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D60275
intl/lwbrk/LineBreaker.cpp
testing/web-platform/meta/css/css-text/word-break/word-break-keep-all-005.html.ini
testing/web-platform/meta/css/css-text/word-break/word-break-keep-all-006.html.ini
testing/web-platform/meta/css/css-text/word-break/word-break-keep-all-007.html.ini
testing/web-platform/meta/css/css-text/word-break/word-break-keep-all-008.html.ini
--- a/intl/lwbrk/LineBreaker.cpp
+++ b/intl/lwbrk/LineBreaker.cpp
@@ -943,16 +943,44 @@ int32_t LineBreaker::Prev(const char16_t
   NS_ASSERTION(aText, "aText shouldn't be null");
   NS_ASSERTION(aLen >= aPos && aPos > 0,
                "Bad position passed to nsJISx4051LineBreaker::Prev");
 
   int32_t prevPos = WordMove(aText, aLen, aPos, -1);
   return prevPos > 0 ? prevPos : NS_LINEBREAKER_NEED_MORE_TEXT;
 }
 
+static bool SuppressBreakForKeepAll(uint32_t aPrev, uint32_t aCh) {
+  auto affectedByKeepAll = [](uint8_t aLBClass) {
+    switch (aLBClass) {
+      // Per https://drafts.csswg.org/css-text-3/#valdef-word-break-keep-all:
+      // "implicit soft wrap opportunities between typographic letter units
+      // (or other typographic character units belonging to the NU, AL, AI,
+      // or ID Unicode line breaking classes [UAX14]) are suppressed..."
+      case U_LB_ALPHABETIC:
+      case U_LB_AMBIGUOUS:
+      case U_LB_NUMERIC:
+      case U_LB_IDEOGRAPHIC:
+      // Additional classes that should be treated similarly, but have been
+      // broken out as separate classes in newer Unicode versions:
+      case U_LB_H2:
+      case U_LB_H3:
+      case U_LB_JL:
+      case U_LB_JV:
+      case U_LB_JT:
+      case U_LB_CONDITIONAL_JAPANESE_STARTER:
+        return true;
+      default:
+        return false;
+    }
+  };
+  return affectedByKeepAll(GetLineBreakClass(aPrev)) &&
+         affectedByKeepAll(GetLineBreakClass(aCh));
+}
+
 void LineBreaker::GetJISx4051Breaks(const char16_t* aChars, uint32_t aLength,
                                     WordBreak aWordBreak, Strictness aLevel,
                                     bool aIsChineseOrJapanese,
                                     uint8_t* aBreakBefore) {
   uint32_t cur;
   int8_t lastClass = CLASS_NONE;
   ContextState state(aChars, aLength);
 
@@ -1006,22 +1034,28 @@ void LineBreaker::GetJISx4051Breaks(cons
         cl = CLASS_BREAKABLE;
       }
     }
 
     bool allowBreak = false;
     if (cur > 0) {
       NS_ASSERTION(CLASS_COMPLEX != lastClass || CLASS_COMPLEX != cl,
                    "Loop should have prevented adjacent complex chars here");
-      if (aWordBreak == WordBreak::Normal ||
-          aWordBreak == WordBreak::BreakAll) {
-        allowBreak = (state.UseConservativeBreaking())
-                         ? GetPairConservative(lastClass, cl)
-                         : GetPair(lastClass, cl);
-      }
+      auto prev = [=]() {
+        char32_t c = aChars[cur - 1];
+        if (cur > 1 && NS_IS_SURROGATE_PAIR(aChars[cur - 2], c)) {
+          c = SURROGATE_TO_UCS4(aChars[cur - 2], c);
+        }
+        return c;
+      };
+      allowBreak =
+          (state.UseConservativeBreaking() ? GetPairConservative(lastClass, cl)
+                                           : GetPair(lastClass, cl)) &&
+          (aWordBreak != WordBreak::KeepAll ||
+           !SuppressBreakForKeepAll(prev(), ch));
     }
     aBreakBefore[cur] = allowBreak;
     if (allowBreak) state.NotifyBreakBefore();
     lastClass = cl;
     if (CLASS_COMPLEX == cl) {
       uint32_t end = cur + chLen;
 
       while (end < aLength) {
@@ -1092,20 +1126,19 @@ void LineBreaker::GetJISx4051Breaks(cons
       if (cls == U_LB_ALPHABETIC || cls == U_LB_NUMERIC ||
           cls == U_LB_COMPLEX_CONTEXT) {
         cl = CLASS_BREAKABLE;
       }
     }
 
     bool allowBreak = false;
     if (cur > 0) {
-      if (aWordBreak == WordBreak::Normal ||
-          aWordBreak == WordBreak::BreakAll) {
-        allowBreak = (state.UseConservativeBreaking())
-                         ? GetPairConservative(lastClass, cl)
-                         : GetPair(lastClass, cl);
-      }
+      allowBreak =
+          (state.UseConservativeBreaking() ? GetPairConservative(lastClass, cl)
+                                           : GetPair(lastClass, cl)) &&
+          (aWordBreak != WordBreak::KeepAll ||
+           !SuppressBreakForKeepAll(aChars[cur - 1], ch));
     }
     aBreakBefore[cur] = allowBreak;
     if (allowBreak) state.NotifyBreakBefore();
     lastClass = cl;
   }
 }
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-text/word-break/word-break-keep-all-005.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[word-break-keep-all-005.html]
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-text/word-break/word-break-keep-all-006.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[word-break-keep-all-006.html]
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-text/word-break/word-break-keep-all-007.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[word-break-keep-all-007.html]
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-text/word-break/word-break-keep-all-008.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[word-break-keep-all-008.html]
-  expected: FAIL