Bug 1549728 - Flush line-breaker whenever the word-break property changes. r=emilio
authorJonathan Kew <jkew@mozilla.com>
Wed, 08 May 2019 15:09:44 +0000
changeset 531897 e663519fd15dfdc0bfa7b5c38163ef110edc3264
parent 531896 aaa0f827616a64aa8c5b347c83f9423fdaa915a7
child 531898 a371ed7348b66078d451235a233f4966a0332f8f
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1549728, 1507744
milestone68.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 1549728 - Flush line-breaker whenever the word-break property changes. r=emilio The patch in bug 1507744 was not sufficient by itself, as the line-breaker could still accumulate a single "current word" across a text-run boundary, and then a single word-break value would be applied to it. We need to flush the line-breaker when word-break changes, so that each part of the word will have break opportunities set according to the appropriate value. Differential Revision: https://phabricator.services.mozilla.com/D30260
dom/base/nsLineBreaker.h
layout/generic/nsTextFrame.cpp
testing/web-platform/meta/css/css-text/word-break/word-break-break-all-inline-004.html.ini
testing/web-platform/meta/css/css-text/word-break/word-break-break-all-inline-005.html.ini
--- a/dom/base/nsLineBreaker.h
+++ b/dom/base/nsLineBreaker.h
@@ -172,17 +172,32 @@ class nsLineBreaker {
    * is breakable whitespace at the end.
    */
   nsresult Reset(bool* aTrailingBreak);
 
   /*
    * Set word-break mode for linebreaker.  This is set by word-break property.
    * @param aMode is LineBreaker::kWordBreak_* value.
    */
-  void SetWordBreak(uint8_t aMode) { mWordBreak = aMode; }
+  void SetWordBreak(uint8_t aMode) {
+    // If current word is non-empty and mode is changing, flush the breaker.
+    if (aMode != mWordBreak && !mCurrentWord.IsEmpty()) {
+      nsresult rv = FlushCurrentWord();
+      if (NS_FAILED(rv)) {
+        NS_WARNING("FlushCurrentWord failed, line-breaks may be wrong");
+      }
+      // If previous mode was break-all, we should allow a break here.
+      // XXX (jfkthame) css-text spec seems unclear on this, raised question in
+      // https://github.com/w3c/csswg-drafts/issues/3897
+      if (mWordBreak == mozilla::intl::LineBreaker::kWordBreak_BreakAll) {
+        mBreakHere = true;
+      }
+    }
+    mWordBreak = aMode;
+  }
 
  private:
   // This is a list of text sources that make up the "current word" (i.e.,
   // run of text which does not contain any whitespace). All the mLengths
   // are are nonzero, these cannot overlap.
   struct TextItem {
     TextItem(nsILineBreakSink* aSink, uint32_t aSinkOffset, uint32_t aLength,
              uint32_t aFlags)
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -2567,45 +2567,48 @@ static bool HasCompressedLeadingWhitespa
   }
   return false;
 }
 
 void BuildTextRunsScanner::SetupBreakSinksForTextRun(gfxTextRun* aTextRun,
                                                      const void* aTextPtr) {
   using mozilla::intl::LineBreaker;
 
-  auto wordBreak =
-    mMappedFlows[0].mStartFrame->StyleText()->EffectiveWordBreak();
-  switch (wordBreak) {
-    case StyleWordBreak::BreakAll:
-      mLineBreaker.SetWordBreak(LineBreaker::kWordBreak_BreakAll);
-      break;
-    case StyleWordBreak::KeepAll:
-      mLineBreaker.SetWordBreak(LineBreaker::kWordBreak_KeepAll);
-      break;
-    case StyleWordBreak::Normal:
-    default:
-      MOZ_ASSERT(wordBreak == StyleWordBreak::Normal);
-      mLineBreaker.SetWordBreak(LineBreaker::kWordBreak_Normal);
-      break;
-  }
-
   // textruns have uniform language
   const nsStyleFont* styleFont = mMappedFlows[0].mStartFrame->StyleFont();
   // We should only use a language for hyphenation if it was specified
   // explicitly.
   nsAtom* hyphenationLanguage =
       styleFont->mExplicitLanguage ? styleFont->mLanguage.get() : nullptr;
   // We keep this pointed at the skip-chars data for the current mappedFlow.
   // This lets us cheaply check whether the flow has compressed initial
   // whitespace...
   gfxSkipCharsIterator iter(aTextRun->GetSkipChars());
 
   for (uint32_t i = 0; i < mMappedFlows.Length(); ++i) {
     MappedFlow* mappedFlow = &mMappedFlows[i];
+    // The CSS word-break value may change within a word, so we reset it for
+    // each MappedFlow. The line-breaker will flush its text if the property
+    // actually changes.
+    auto wordBreak =
+      mappedFlow->mStartFrame->StyleText()->EffectiveWordBreak();
+    switch (wordBreak) {
+      case StyleWordBreak::BreakAll:
+        mLineBreaker.SetWordBreak(LineBreaker::kWordBreak_BreakAll);
+        break;
+      case StyleWordBreak::KeepAll:
+        mLineBreaker.SetWordBreak(LineBreaker::kWordBreak_KeepAll);
+        break;
+      case StyleWordBreak::Normal:
+      default:
+        MOZ_ASSERT(wordBreak == StyleWordBreak::Normal);
+        mLineBreaker.SetWordBreak(LineBreaker::kWordBreak_Normal);
+        break;
+    }
+
     uint32_t offset = iter.GetSkippedOffset();
     gfxSkipCharsIterator iterNext = iter;
     iterNext.AdvanceOriginal(mappedFlow->GetContentEnd() -
                              mappedFlow->mStartFrame->GetContentOffset());
 
     UniquePtr<BreakSink>* breakSink = mBreakSinks.AppendElement(
         MakeUnique<BreakSink>(aTextRun, mDrawTarget, offset));
     if (!breakSink || !*breakSink) return;
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-text/word-break/word-break-break-all-inline-004.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[word-break-break-all-inline-004.html]
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-text/word-break/word-break-break-all-inline-005.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[word-break-break-all-inline-005.html]
-  expected: FAIL