Bug 1623764 - Part 4: Stop at hard linebreaks when eat_space_to_next_word r=emilio,masayuki

Differential Revision: https://phabricator.services.mozilla.com/D85923

editor/libeditor/tests/test_selection_move_commands.html

@@ -132,7 +132,7 @@ async function* runTests() {
   yield;
   testScrollCommand("cmd_scrollLineUp", root.scrollHeight - 100 - lineHeight);
 
-  var runSelectionTests = function(selectWordNextNode, selectWordNextOffset) {
+  var runSelectionTests = function() {
     testMoveCommand("cmd_moveBottom", body, 23);
     testMoveCommand("cmd_moveTop", node(0), 0);
     testSelectCommand("cmd_selectBottom", body, 23);
@@ -165,7 +165,7 @@ async function* runTests() {
     testMoveCommand("cmd_wordNext", body, 23);
     testSelectCommand("cmd_selectWordPrevious", node(22), 0);
     SpecialPowers.doCommand(window, "cmd_moveTop");
-    testSelectCommand("cmd_selectWordNext", selectWordNextNode, selectWordNextOffset);
+    testSelectCommand("cmd_selectWordNext", body, 1);
 
     SpecialPowers.doCommand(window, "cmd_moveTop");
     var lineNum = testPageMoveCommand("cmd_movePageDown", 0);
@@ -182,9 +182,9 @@ async function* runTests() {
   };
 
   await SpecialPowers.pushPrefEnv({set: [["layout.word_select.eat_space_to_next_word", false]]});
-  runSelectionTests(body, 1);
+  runSelectionTests();
   await SpecialPowers.pushPrefEnv({set: [["layout.word_select.eat_space_to_next_word", true]]});
-  runSelectionTests(node(2), 0);
+  runSelectionTests();
 }
 
 function cleanup() {

layout/generic/nsIFrame.cpp

@@ -8430,6 +8430,9 @@ nsresult nsIFrame::PeekOffsetForCharacter(nsPeekOffsetStruct* aPos,
 nsresult nsIFrame::PeekOffsetForWord(nsPeekOffsetStruct* aPos,
                                      int32_t aOffset) {
   SelectablePeekReport current{this, aOffset};
+  bool shouldStopAtHardBreak =
+      aPos->mWordMovementType == eDefaultBehavior &&
+      StaticPrefs::layout_word_select_eat_space_to_next_word();
   bool wordSelectEatSpace = ShouldWordSelectionEatSpace(*aPos);
 
   PeekWordState state;
@@ -8464,11 +8467,35 @@ nsresult nsIFrame::PeekOffsetForWord(nsPeekOffsetStruct* aPos,
       break;
     }
 
+    if (shouldStopAtHardBreak && next.mJumpedHardBreak) {
+      /**
+       * Prev, always: Jump and stop right there
+       * Next, saw inline: just stop
+       * Next, no inline: Jump and consume whitespaces
+       */
+      if (aPos->mDirection == eDirPrevious) {
+        // Try moving to the previous line if exists
+        current.TransferTo(*aPos);
+        current.mFrame->PeekOffsetForCharacter(aPos, current.mOffset);
+        return NS_OK;
+      }
+      if (state.mSawInlineCharacter || current.mJumpedHardBreak) {
+        if (current.mFrame->HasSignificantTerminalNewline()) {
+          current.mOffset -= 1;
+        }
+        current.TransferTo(*aPos);
+        return NS_OK;
+      }
+      // Mark the state as whitespace and continue
+      state.Update(false, true);
+    }
+
     if (next.mJumpedLine) {
       state.mContext.Truncate();
     }
     current = next;
     // Jumping a line is equivalent to encountering whitespace
+    // This affects only when it already met an actual character
     if (wordSelectEatSpace && next.mJumpedLine) {
       state.SetSawBeforeType();
     }
@@ -8912,6 +8939,12 @@ nsIFrame::SelectablePeekReport nsIFrame::GetFrameFromDirection(
       if (!aJumpLines) {
         return result;  // we are done. cannot jump lines
       }
+      int32_t lineToCheckWrap =
+          aDirection == eDirPrevious ? thisLine - 1 : thisLine;
+      if (lineToCheckWrap < 0 ||
+          !it->GetLine(lineToCheckWrap).unwrap().mIsWrapped) {
+        result.mJumpedHardBreak = true;
+      }
     }
 
     traversedFrame = frameTraversal->Traverse(aDirection == eDirNext);
@@ -8934,7 +8967,9 @@ nsIFrame::SelectablePeekReport nsIFrame::GetFrameFromDirection(
       for (nsIFrame* current = traversedFrame->GetPrevSibling(); current;
            current = current->GetPrevSibling()) {
         if (!current->IsBlockOutside() && IsSelectable(current)) {
-          canSkipBr = true;
+          if (!current->IsBrFrame()) {
+            canSkipBr = true;
+          }
           break;
         }
       }

layout/generic/nsIFrame.h

@@ -3789,8 +3789,10 @@ class nsIFrame : public nsQueryFrame {
      * indicates that we arrived at its end.
      */
     int32_t mOffset = 0;
-    /** whether this frame and the returned frame are on different lines */
+    /** whether the input frame and the returned frame are on different lines */
     bool mJumpedLine = false;
+    /** whether we met a hard break between the input and the returned frame */
+    bool mJumpedHardBreak = false;
     /** whether we jumped over a non-selectable frame during the search */
     bool mMovedOverNonSelectableText = false;
 

layout/generic/test/mochitest.ini

@@ -103,6 +103,7 @@ support-files = file_bug1307853.html
 [test_bug1499961.html]
 [test_bug1566783.html]
 support-files = file_bug1566783.html
+[test_bug1623764.html]
 [test_bug1642588.html]
 [test_bug1644511.html]
 [test_contained_plugin_transplant.html]

layout/generic/test/test_bug1623764.html

@@ -0,0 +1,292 @@
+<!DOCTYPE html>
+<title>Test Windows conventional caret movement behavior</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<link rel="stylesheet" href="/tests/SimpleTest/test.css" />
+<style>
+  [contenteditable] {
+    font-size: 14px;
+    font-family: monospace;
+    overflow-wrap: normal;
+    outline: none;
+    width: 35ch;
+  }
+  textarea {
+    font-size: 14px;
+    font-family: monospace;
+  }
+
+  .break-word {
+    overflow-wrap: break-word;
+  }
+</style>
+<div id="testDiv" contenteditable></div>
+<textarea id="testTextarea" cols=35></textarea>
+
+<script>
+// Call testEach(cases[i]) on console when debugging
+/**
+ * @type {TestCase[]}
+ *
+ * @typedef {object} TestCase
+ * @property {string} title
+ * @property {string} text Text on which the test runs.
+ *   The newlines and spaces will be auto-converted if needed.
+ * @property {boolean} [reverse] Test in both forward and backward directions
+ * @property {boolean} [backward] Move backward by cmd_wordPrevious
+ * @property {ElementSpecific} [div] Can be omitted when there is a bug
+ * @property {ElementSpecific} textarea
+ *
+ * @typedef {object} ElementSpecific
+ * @property {number} expectedOffset Expected offset after a caret move
+ * @property {number} [expectedNodeIndex] The index of child node with focus after a caret move.
+ *   -1 means the parent node.
+ * @property {number} [initialOffset=0] initialOffset offset before a caret move
+ * @property {number} [initialNodeIndex]
+ */
+
+const kFirstWordLength = "Supercalifragilisticexpialidocious".length; // 34
+const kParentNodeIndex = -1; // special value
+const cases = [
+  {
+    title: "Eats inline whitespaces after word",
+    text: "Supercalifragilisticexpialidocious foo bar fussball",
+    reverse: true,
+    div: {
+      expectedOffset: kFirstWordLength + 1
+    },
+    textarea: {
+      expectedOffset: kFirstWordLength + 1
+    }
+  },
+  {
+    title: "Eats inline whitespaces across a wrapped line after word",
+    text: "Supercalifragilisticexpialidocious       foo bar fussball",
+    reverse: true,
+    div: {
+      expectedOffset: kFirstWordLength + 7
+    },
+    textarea: {
+      expectedOffset: kFirstWordLength + 7
+    }
+  },
+  {
+    title: "Eats inline whitespaces across multiple wrapped lines after word",
+    text: `Supercalifragilisticexpialidocious                                                                 foo bar fussball`,
+    reverse: true,
+    div: {
+      expectedOffset: kFirstWordLength + 65
+    },
+    textarea: {
+      expectedOffset: kFirstWordLength + 65
+    }
+  },
+  {
+    title: "Eats inline whitespaces after a whole word and stop before a newline",
+    text: "Supercalifragilisticexpialidocious \nfoo bar fussball",
+    reverse: true,
+    div: {
+      expectedOffset: 1,
+      expectedNodeIndex: kParentNodeIndex
+    },
+    textarea: {
+      expectedOffset: kFirstWordLength + 1
+    }
+  },
+  {
+    title: "Eats inline whitespaces after a partial word and stop before a newline",
+    text: "Supercalifragilisticexpialidocious \nfoo bar fussball",
+    div: {
+      initialOffset: 5,
+      expectedOffset: 1,
+      expectedNodeIndex: kParentNodeIndex
+    },
+    textarea: {
+      initialOffset: 5,
+      expectedOffset: kFirstWordLength + 1
+    }
+  },
+  {
+    title: "Eats inline whitespaces without a word and stop before a newline",
+    text: "Supercalifragilisticexpialidocious \n      foo bar fussball",
+    // TODO(krosylight): Currently ClusterIterator internally skips trailing whitespace
+    // div: {
+    //   initialOffset: kFirstWordLength,
+    //   expectedOffset: 1,
+    //   expectedNodeIndex: kParentNodeIndex
+    // },
+    textarea: {
+      initialOffset: kFirstWordLength,
+      expectedOffset: kFirstWordLength + 1
+    }
+  },
+  {
+    title: "Jumps to the next line and eats inline whitespaces",
+    text: "Supercalifragilisticexpialidocious \n      foo bar fussball",
+    reverse: true,
+    div: {
+      initialOffset: kFirstWordLength + 1,
+      expectedOffset: 6,
+      expectedNodeIndex: 2
+    },
+    textarea: {
+      initialOffset: kFirstWordLength + 1,
+      expectedOffset: kFirstWordLength + 8
+    }
+  },
+  {
+    title: "Stops on whitespaces after word",
+    text: "Supercalifragilisticexpialidocious \n      foo bar fussball",
+    backward: true,
+    div: {
+      initialOffset: 8,
+      initialNodeIndex: 2,
+      expectedOffset: 6,
+      expectedNodeIndex: 2
+    },
+    textarea: {
+      initialOffset: 44, // middle of "foo"
+      expectedOffset: 42
+    }
+  },
+  // TODO(krosylight): Currently no way to tell a word break is from line wrapping
+  // {
+  //   title: "Ignore a word break introduced by line wrapping",
+  //   text: "Supercalifragilisticexpialidociouspostfix",
+  //   className: "narrow break-word",
+  //   div: {
+  //     expectedOffset: kFirstWordLength + 7
+  //   },
+  //   textarea: {
+  //     expectedOffset: kFirstWordLength + 7
+  //   }
+  // }
+  {
+    title: "Jump only one line at an empty line end",
+    text: "Supercalifragilisticexpialidocious \n\nfoo bar fussball",
+    reverse: true,
+    div: {
+      initialOffset: 2,
+      initialNodeIndex: kParentNodeIndex,
+      expectedOffset: 0,
+      expectedNodeIndex: 3
+    },
+    textarea: {
+      initialOffset: kFirstWordLength + 2,
+      expectedOffset: kFirstWordLength + 3
+    }
+  },
+  {
+    title: "Jump only one line at a non-empty line end",
+    text: "Supercalifragilisticexpialidocious \n\nfoo bar fussball",
+    reverse: true,
+    div: {
+      initialOffset: kFirstWordLength + 1,
+      expectedOffset: 2,
+      expectedNodeIndex: -1
+    },
+    textarea: {
+      initialOffset: kFirstWordLength + 1,
+      expectedOffset: kFirstWordLength + 2
+    }
+  },
+];
+
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(async () => {
+  await SpecialPowers.pushPrefEnv({
+    set: [["layout.word_select.eat_space_to_next_word", true]]
+  });
+  // Test on div first to prevent Bug 1623413
+  for (const testCase of cases) {
+    if (testCase.div) {
+      testOnDiv(testCase);
+    }
+  }
+  for (const testCase of cases) {
+    testOnTextarea(testCase);
+  }
+  SimpleTest.finish();
+});
+
+/** @param {TestCase} testCase */
+function testOnDiv(testCase) {
+  const {
+    title,
+    backward = false,
+    reverse = false,
+  } = testCase;
+  const {
+    initialOffset = 0,
+    expectedOffset,
+  } = testCase.div;
+  if (expectedOffset === undefined) {
+    throw new Error("`expectedOffset` must be defined.")
+  }
+
+  testDiv.innerHTML = testCase.text.replaceAll(/ {2}/g, " &nbsp;").replaceAll(/\n/g, "<br>");
+  const initialNode = childNode(testDiv, testCase.div.initialNodeIndex);
+  const expectedNode = childNode(testDiv, testCase.div.expectedNodeIndex);
+
+  const selection = getSelection();
+  selection.collapse(initialNode, initialOffset);
+
+  moveByWord(backward);
+
+  is(selection.focusNode, expectedNode, `focusNode in "${title}"`);
+  is(selection.focusOffset, expectedOffset, `focusOffset in "${title}"`);
+
+  if (reverse) {
+    selection.collapse(expectedNode, expectedOffset);
+
+    moveByWord(!backward);
+
+    is(selection.focusNode, initialNode, `focusNode with reversed selection in "${title}"`);
+    is(selection.focusOffset, initialOffset, `focusOffset with reversed selection in "${title}"`);
+  }
+}
+
+function testOnTextarea(testCase) {
+  const {
+    title,
+    backward = false,
+    reverse = false,
+  } = testCase;
+  const {
+    initialOffset = 0,
+    expectedOffset,
+  } = testCase.textarea;
+  if (expectedOffset === undefined) {
+    throw new Error("`expectedOffset` must be defined.")
+  }
+
+  testTextarea.value = testCase.text;
+  testTextarea.selectionStart = testTextarea.selectionEnd = initialOffset;
+  testTextarea.focus();
+
+  moveByWord(backward);
+
+  is(testTextarea.selectionStart, expectedOffset, `selectionStart in "${title}"`);
+
+  if (reverse) {
+    testTextarea.selectionStart = testTextarea.selectionEnd = expectedOffset;
+
+    moveByWord(!backward);
+
+    is(testTextarea.selectionStart, initialOffset, `selectionStart with reversed selection in "${title}"`);
+  }
+}
+
+function childNode(parent, index = 0) {
+  if (index === kParentNodeIndex) {
+    return parent;
+  }
+  return parent.childNodes[index];
+}
+
+/** @param {boolean} backward */
+function moveByWord(backward) {
+  const dir = backward ? "Previous" : "Next";
+  SpecialPowers.doCommand(window, `cmd_word${dir}`);
+}
+</script>