Revert "Bug 1987845 - Text Fragments: Correctly deal with punctuation when identifying the first/last word of the target range. r=smaug" for causing windows nightly as release build bustages on TextDirectiveCreator.cpp.

This reverts commit 9d7b876.

Author: IulianMoraru

dom/base/TextDirectiveCreator.cpp

@@ -380,10 +380,20 @@ void RangeBasedTextDirectiveCreator::CollectContextTermWordBoundaryDistances() {
     mStartWordEndDistances.Clear();
     TEXT_FRAGMENT_LOG("Start term cannot be extended.");
   } else {
-    mStartFirstWordLengthIncludingWhitespace =
-        TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-            TextScanDirection::Right>(mStartFoldCaseContent,
-                                      mStartWordEndDistances);
+    // Find the start position for the second word, which is used as the base
+    // for the word end distance.
+    auto [firstWordEndPos, secondWordBeginPos] =
+        intl::WordBreaker::FindWord(mStartContent, mStartWordEndDistances[0]);
+    MOZ_DIAGNOSTIC_ASSERT(firstWordEndPos == mStartWordEndDistances[0]);
+    mStartFirstWordLengthIncludingWhitespace = secondWordBeginPos;
+    mStartFoldCaseContent = Substring(mStartFoldCaseContent,
+                                      mStartFirstWordLengthIncludingWhitespace);
+    mStartWordEndDistances.RemoveElementAt(0);
+    for (auto& distance : mStartWordEndDistances) {
+      MOZ_DIAGNOSTIC_ASSERT(distance >=
+                            mStartFirstWordLengthIncludingWhitespace);
+      distance = distance - mStartFirstWordLengthIncludingWhitespace;
+    }
     TEXT_FRAGMENT_LOG(
         "Word end distances for start term, starting at the beginning of the "
         "second word: {}",
@@ -413,9 +423,15 @@ void RangeBasedTextDirectiveCreator::CollectContextTermWordBoundaryDistances() {
     MOZ_DIAGNOSTIC_ASSERT(lastWordBeginPos ==
                           mEndContent.Length() - mEndWordBeginDistances[0]);
     mEndLastWordLengthIncludingWhitespace =
-        TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-            TextScanDirection::Left>(mEndFoldCaseContent,
-                                     mEndWordBeginDistances);
+        mEndContent.Length() - secondLastWordEndPos;
+
+    mEndFoldCaseContent =
+        Substring(mEndFoldCaseContent, 0, secondLastWordEndPos);
+    mEndWordBeginDistances.RemoveElementAt(0);
+    for (auto& distance : mEndWordBeginDistances) {
+      MOZ_DIAGNOSTIC_ASSERT(distance >= mEndLastWordLengthIncludingWhitespace);
+      distance = distance - mEndLastWordLengthIncludingWhitespace;
+    }
     TEXT_FRAGMENT_LOG(
         "Word begin distances for end term, starting at the end of the second "
         "last word: {}",

dom/base/TextDirectiveUtil.h

@@ -225,30 +225,6 @@ class TextDirectiveUtil final {
   static bool WordIsJustWhitespaceOrPunctuation(const nsAString& aString,
                                                 uint32_t aWordBegin,
                                                 uint32_t aWordEnd);
-
-  /**
-   * @brief Finds the position of the beginning of the second word (in
-   *        `direction`), then removes everything up to that position from
-   *       `aString` and `aWordDistances`.
-   *
-   * This function modifies both `aString` and `aWordDistances`.
-   * It expects `aString` to be non-empty, and to contain at least two words,
-   * as indicated by `aWordDistances` containing at least two elements.
-   *
-   * @tparam direction Either left-to-right or right-to-left.
-   * @param aString        The string to modify. Must not be empty.
-   * @param aWordDistances The array of word boundary distances. The distances
-   *                       are always sorted and contain monotonically
-   *                       increasing values. For LTR, the distances are based
-   *                       off the beginning of the string. For RTL, the
-   *                       distances are based off the end of the string. Must
-   *                       contain at least two elements.
-   * @return The length of the first word including whitespace and
-   *         punctuation up to the beginning of the second word.
-   */
-  template <TextScanDirection direction>
-  static uint32_t RemoveFirstWordFromStringAndDistanceArray(
-      nsAString& aString, nsTArray<uint32_t>& aWordDistances);
 };
 
 class TimeoutWatchdog final {
@@ -671,50 +647,6 @@ template <TextScanDirection direction>
   return commonLength;
 }
 
-template <TextScanDirection direction>
-/*static*/ uint32_t
-TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray(
-    nsAString& aString, nsTArray<uint32_t>& aWordDistances) {
-  MOZ_DIAGNOSTIC_ASSERT(!aString.IsEmpty());
-  MOZ_DIAGNOSTIC_ASSERT(aWordDistances.Length() > 1);
-  auto lengthOfFirstWordPlusWhitespaceAndPunctuation = aWordDistances[0];
-  auto chIsWhitespaceOrPunctuation = [&](uint32_t distance) {
-    const char16_t ch = aString.CharAt(direction == TextScanDirection::Right
-                                           ? distance
-                                           : aString.Length() - distance - 1);
-    return nsContentUtils::IsHTMLWhitespace(ch) ||
-           mozilla::IsPunctuationForWordSelect(ch);
-  };
-  while (lengthOfFirstWordPlusWhitespaceAndPunctuation < aString.Length() &&
-         chIsWhitespaceOrPunctuation(
-             lengthOfFirstWordPlusWhitespaceAndPunctuation)) {
-    ++lengthOfFirstWordPlusWhitespaceAndPunctuation;
-  }
-  if (lengthOfFirstWordPlusWhitespaceAndPunctuation == aString.Length()) {
-    // In this case the string only contains whitespace or punctuation after the
-    // first word.
-    aWordDistances.Clear();
-    return lengthOfFirstWordPlusWhitespaceAndPunctuation;
-  }
-  // Adjust all distances to be relative to the new start position.
-  // In the case that the loop above jumps over punctuation which is actually
-  // considered to be a word, the distance underflows (or becomes zero).
-  // These obsolete distances are then removed.
-  for (auto& wordDistance : aWordDistances) {
-    wordDistance -= lengthOfFirstWordPlusWhitespaceAndPunctuation;
-  }
-  aWordDistances.RemoveElementsBy([&aString](uint32_t distance) {
-    return distance == 0 || distance > aString.Length();
-  });
-  if constexpr (direction == TextScanDirection::Right) {
-    aString = Substring(aString, lengthOfFirstWordPlusWhitespaceAndPunctuation);
-  } else {
-    aString = Substring(
-        aString, 0,
-        aString.Length() - lengthOfFirstWordPlusWhitespaceAndPunctuation);
-  }
-  return lengthOfFirstWordPlusWhitespaceAndPunctuation;
-}
 }  // namespace mozilla::dom
 
 #endif

dom/base/test/gtest/TestTextDirective.cpp

@@ -50,29 +50,6 @@ TEST(TestTextDirective, ComputeWordBoundaryDistancesPunctuationOnly)
   EXPECT_EQ(wordEndDistances[0], 5u);
 }
 
-TEST(TestTextDirective,
-     ComputeWordBoundaryDistancesWithPunctuationSeparatedByWhitespace)
-{
-  nsString text(u"foo ... bar");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Right>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 2u);
-  EXPECT_EQ(wordEndDistances[0], 3u);   // "foo"
-  EXPECT_EQ(wordEndDistances[1], 11u);  // "foo ... bar"
-}
-
-TEST(TestTextDirective, ComputeWordBoundaryDistancesEndingInPunctuation)
-{
-  nsString text(u"foo ...");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Right>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 2u);
-  EXPECT_EQ(wordEndDistances[0], 3u);  // "foo"
-  EXPECT_EQ(wordEndDistances[1], 7u);  // "foo ..."
-}
-
 TEST(TestTextDirective, ComputeWordBoundaryDistancesWithEmptyString)
 {
   nsString text(u"");
@@ -82,128 +59,3 @@ TEST(TestTextDirective, ComputeWordBoundaryDistancesWithEmptyString)
   EXPECT_EQ(wordEndDistances.Length(), 1u);
   EXPECT_EQ(wordEndDistances[0], 0u);
 }
-
-TEST(TestTextDirective,
-     RemoveFirstWordFromStringAndDistanceArrayPunctuationAfterFirstWordLTR)
-{
-  nsString text(u"Hello, world! This is a test.");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Right>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 7u);
-  uint32_t firstWordLength =
-      TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-          TextScanDirection::Right>(text, wordEndDistances);
-  EXPECT_EQ(firstWordLength, 7u);  // "Hello, "
-  EXPECT_EQ(text, u"world! This is a test.");
-  EXPECT_EQ(wordEndDistances.Length(), 6u);
-  EXPECT_EQ(wordEndDistances[0], 5u);   // "world"
-  EXPECT_EQ(wordEndDistances[1], 11u);  // "world! This"
-  EXPECT_EQ(wordEndDistances[2], 14u);  // "world! This is"
-  EXPECT_EQ(wordEndDistances[3], 16u);  // "world! This is a"
-  EXPECT_EQ(wordEndDistances[4], 21u);  // "world! This is a test"
-  EXPECT_EQ(wordEndDistances[5], 22u);  // "world! This is a test."
-}
-
-TEST(TestTextDirective, RemoveFirstWordFromStringInParenthesisLTR)
-{
-  nsString text(u"(Hello) world");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Right>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 2u);
-  uint32_t firstWordLength =
-      TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-          TextScanDirection::Right>(text, wordEndDistances);
-  EXPECT_EQ(firstWordLength, 8u);  // "(Hello) "
-  EXPECT_EQ(text, u"world");
-  EXPECT_EQ(wordEndDistances.Length(), 1u);
-  EXPECT_EQ(wordEndDistances[0], 5u);  // "world"
-}
-
-TEST(TestTextDirective, RemoveFirstWordFromStringAndDistanceArrayRTL)
-{
-  nsString text(u"Hello, world! This is a test.");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Left>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 6u);
-  uint32_t firstWordLength =
-      TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-          TextScanDirection::Left>(text, wordEndDistances);
-  EXPECT_EQ(firstWordLength, 6u);  // " test."
-  EXPECT_EQ(text, u"Hello, world! This is a");
-  EXPECT_EQ(wordEndDistances.Length(), 5u);
-  EXPECT_EQ(wordEndDistances[0], 1u);   // "a"
-  EXPECT_EQ(wordEndDistances[1], 4u);   // "is a"
-  EXPECT_EQ(wordEndDistances[2], 9u);   // "This is a"
-  EXPECT_EQ(wordEndDistances[3], 16u);  // "world! This is a"
-  EXPECT_EQ(wordEndDistances[4], 23u);  // "Hello, world! This is a"
-}
-
-TEST(TestTextDirective, RemoveFirstWordFromStringInParenthesisRTL)
-{
-  nsString text(u"Hello (world)");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Left>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 2u);
-  uint32_t firstWordLength =
-      TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-          TextScanDirection::Left>(text, wordEndDistances);
-  EXPECT_EQ(firstWordLength, 8u);  // " (world)"
-  EXPECT_EQ(text, u"Hello");
-  EXPECT_EQ(wordEndDistances.Length(), 1u);
-  EXPECT_EQ(wordEndDistances[0], 5u);  // "Hello"
-}
-
-TEST(TestTextDirective,
-     RemoveFirstWordFromStringAndDistanceArrayMultiplePunctuationLTR)
-{
-  nsString text(u"...foo!!! bar?");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Right>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 3u);
-  uint32_t firstWordLength =
-      TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-          TextScanDirection::Right>(text, wordEndDistances);
-  EXPECT_EQ(firstWordLength, 10u);  // "...foo!!! "
-  EXPECT_EQ(text, u"bar?");
-  EXPECT_EQ(wordEndDistances.Length(), 2u);
-  EXPECT_EQ(wordEndDistances[0], 3u);  // "bar"
-  EXPECT_EQ(wordEndDistances[1], 4u);  // "bar?"
-}
-
-TEST(TestTextDirective,
-     RemoveFirstWordFromStringAndDistanceArrayMultiplePunctuationRTL)
-{
-  nsString text(u"foo!!! ...bar");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Left>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 2u);
-  uint32_t firstWordLength =
-      TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-          TextScanDirection::Left>(text, wordEndDistances);
-  EXPECT_EQ(firstWordLength, 10u);  // "!!! ...bar"
-  EXPECT_EQ(text, u"foo");
-  EXPECT_EQ(wordEndDistances.Length(), 1u);
-  EXPECT_EQ(wordEndDistances[0], 3u);  // "foo"
-}
-
-TEST(TestTextDirective,
-     RemoveFirstWordFromStringAndDistanceArrayEndsInPunctuationLTR)
-{
-  nsString text(u"foo ...");
-  nsTArray<uint32_t> wordEndDistances =
-      TextDirectiveUtil::ComputeWordBoundaryDistances<TextScanDirection::Right>(
-          text);
-  EXPECT_EQ(wordEndDistances.Length(), 2u);
-  uint32_t firstWordLength =
-      TextDirectiveUtil::RemoveFirstWordFromStringAndDistanceArray<
-          TextScanDirection::Right>(text, wordEndDistances);
-  EXPECT_EQ(firstWordLength, 7u);  // "foo ..."
-  EXPECT_EQ(text, u"foo ...");
-  EXPECT_EQ(wordEndDistances.Length(), 0u);
-}

dom/base/test/test_text-fragments-create-text-directive.html

@@ -72,12 +72,10 @@
       <td>foo</td>
     </tr>
   </table>
-  <!-- (crash)Test cases for Bug 1987845; example paragraph found on https://en.wikipedia.org/wiki/Nuclear_weapon-->
+  <!-- (crash)Test case for Bug 1987845; example paragraph found on https://en.wikipedia.org/wiki/Nuclear_weapon-->
   <p>Nuclear<!-- This is needed so that there is an additional match to consider --></p>
   <p id="rangeBasedWihSpaceInTheMiddle">Nuclear weapons have only twice been used in warfare, both times by the United States against Japan at the end of World War II. On August 6, 1945, the United States Army Air Forces (USAAF) detonated a uranium gun-type fission bomb nicknamed "Little Boy" over the Japanese city of Hiroshima; three days later, on August 9, the USAAF[4] detonated a plutonium implosion-type fission bomb nicknamed "Fat Man" over the Japanese city of Nagasaki. These bombings caused injuries that resulted in the deaths of approximately 200,000 civilians and military personnel.[5] The ethics of these bombings and their role in Japan's surrender are to this day, still subjects of debate</p>
-  <!-- note that "text" must already exist in the page to trigger this. -->
-  <p id="rangeBasedWithPunctuationAfterFirstWordStart">Text: the colon</p>
-  <p id="rangeBasedWithPunctuationAfterFirstWordEnd">is important here</p>
+
   <script>
     document.addEventListener("DOMContentLoaded", () => {
       const empty = document.createTextNode("");
@@ -454,15 +452,6 @@
           endOffset: rangeBasedWihSpaceInTheMiddle.firstChild.length,
           content: "Nuclear weapons have only twice been used in warfare, both times by the United States against Japan at the end of World War II. On August 6, 1945, the United States Army Air Forces (USAAF) detonated a uranium gun-type fission bomb nicknamed \"Little Boy\" over the Japanese city of Hiroshima; three days later, on August 9, the USAAF[4] detonated a plutonium implosion-type fission bomb nicknamed \"Fat Man\" over the Japanese city of Nagasaki. These bombings caused injuries that resulted in the deaths of approximately 200,000 civilians and military personnel.[5] The ethics of these bombings and their role in Japan's surrender are to this day, still subjects of debate",
           textDirective: "text=Nuclear%20weapons,debate",
-        },
-        {
-          name: "Test for Bug 1987845: Punctuation after first word of range",
-          startContainer: rangeBasedWithPunctuationAfterFirstWordStart.firstChild,
-          startOffset: 0,
-          endContainer: rangeBasedWithPunctuationAfterFirstWordEnd.firstChild,
-          endOffset: rangeBasedWithPunctuationAfterFirstWordEnd.firstChild.length,
-          content: "Text: the colon\n  is important here",
-          textDirective: "text=Text%3A%20the,here",
         }
       ]) {
         const range = document.createRange();