Bug 1676785 - Simplify XUL special code-path for <input> value setter. r=masayuki

This allows us to remove nsISelectionController.selectAll() and related
code too, and should not change behavior.

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

dom/base/nsISelectionController.idl

@@ -276,10 +276,6 @@ interface nsISelectionController : nsISelectionDisplay
    */
     void scrollCharacter(in boolean right);
 
-  /** SelectAll will select the whole page
-   */
-    void selectAll();
-
   /** CheckVisibility will return true if textnode and offsets are actually rendered
    *  in the current precontext.
    *  @param aNode textNode to test

dom/base/test/chrome/chrome.ini

@@ -69,6 +69,7 @@ support-files =
   file_document-element-inserted-inner.xhtml
 [test_domparsing.xhtml]
 [test_fileconstructor.xhtml]
+[test_input_value_set_preserve_undo.xhtml]
 [test_nsITextInputProcessor.xhtml]
 [test_permission_isHandlingUserInput.xhtml]
 support-files = ../dummy.html

dom/base/test/chrome/test_input_value_set_preserve_undo.xhtml

@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+<window title="Bug 1676785"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+  xmlns:html="http://www.w3.org/1999/xhtml"
+  xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+<script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+<script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
+<html:body>
+  <xul:hbox>
+    <html:input id="xul" />
+  </xul:hbox>
+  <html:div>
+    <html:input id="non-xul" />
+  </html:div>
+</html:body>
+<script class="testbody">
+SimpleTest.waitForExplicitFinish();
+
+function shouldPreserveHistory(input, preserve) {
+  input.focus();
+  input.value = "abc";
+  input.value = "def";
+  synthesizeKey("z", {ctrlKey: true});
+  (preserve ? is : isnot)(input.value, "abc", `Expected ${input.id} to ${preserve ? "" : "not "}preserve undo history when setting .value`);
+}
+
+window.onload = function() {
+  shouldPreserveHistory(document.getElementById("xul"), true);
+  shouldPreserveHistory(document.getElementById("non-xul"), false);
+
+  SimpleTest.finish();
+}
+</script>
+</window>

dom/html/HTMLInputElement.cpp

@@ -2641,10 +2641,14 @@ nsresult HTMLInputElement::SetValueInternal(const nsAString& aValue,
 
   // We want to remember if the SetValueInternal() call is being made for a XUL
   // element.  We do that by looking at the parent node here, and if that node
-  // is a XUL node, we consider our control a XUL control.
+  // is a XUL node, we consider our control a XUL control. XUL controls preserve
-  nsIContent* parent = GetParent();
+  // edit history across value setters.
-  if (parent && parent->IsXULElement()) {
+  //
-    aFlags |= TextControlState::eSetValue_ForXUL;
+  // TODO(emilio): Rather than doing this maybe add an attribute instead and
+  // read it only on chrome docs or something? That'd allow front-end code to
+  // move away from xul without weird side-effects.
+  if (mParent && mParent->IsXULElement()) {
+    aFlags |= TextControlState::eSetValue_PreserveHistory;
   }
 
   switch (GetValueMode()) {

dom/html/TextControlState.cpp

@@ -347,7 +347,6 @@ class TextInputSelectionController final : public nsSupportsWeakReference,
   NS_IMETHOD ScrollPage(bool aForward) override;
   NS_IMETHOD ScrollLine(bool aForward) override;
   NS_IMETHOD ScrollCharacter(bool aRight) override;
-  NS_IMETHOD SelectAll(void) override;
   NS_IMETHOD CheckVisibility(nsINode* node, int16_t startOffset,
                              int16_t EndOffset, bool* _retval) override;
   virtual nsresult CheckVisibilityContent(nsIContent* aNode,
@@ -752,15 +751,6 @@ TextInputSelectionController::ScrollCharacter(bool aRight) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-TextInputSelectionController::SelectAll() {
-  if (!mFrameSelection) {
-    return NS_ERROR_NULL_POINTER;
-  }
-  RefPtr<nsFrameSelection> frameSelection = mFrameSelection;
-  return frameSelection->SelectAll();
-}
-
 void TextInputSelectionController::SelectionWillTakeFocus() {
   if (mFrameSelection) {
     if (PresShell* shell = mFrameSelection->GetPresShell()) {
@@ -2776,61 +2766,16 @@ bool TextControlState::SetValueWithTextEditor(
   // by script.
   AutoInputEventSuppresser suppressInputEventDispatching(textEditor);
 
-  if (aHandlingSetValue.GetSetValueFlags() & eSetValue_ForXUL) {
-    // On XUL <textbox> element, we need to preserve existing undo
-    // transactions.
-    // XXX Do we really need to do such complicated optimization?
-    //     This was landed for web pages which set <textarea> value
-    //     per line (bug 518122).  For example:
-    //       for (;;) {
-    //         textarea.value += oneLineText + "\n";
-    //       }
-    //     However, this path won't be used in web content anymore.
-    nsCOMPtr<nsISelectionController> kungFuDeathGrip = mSelCon.get();
-    // Use nsString to avoid copying string buffer in most cases.
-    nsString currentValue;
-    if (aHandlingSetValue.GetOldValue()) {
-      currentValue.Assign(*aHandlingSetValue.GetOldValue());
-    } else {
-      mBoundFrame->GetText(currentValue);
-    }
-    uint32_t currentLength = currentValue.Length();
-    uint32_t newlength = aHandlingSetValue.GetSettingValue().Length();
-    if (!currentLength ||
-        !StringBeginsWith(aHandlingSetValue.GetSettingValue(), currentValue)) {
-      // Replace the whole text.
-      currentLength = 0;
-      kungFuDeathGrip->SelectAll();
-    } else {
-      // Collapse selection to the end so that we can append data.
-      mBoundFrame->SelectAllOrCollapseToEndOfText(false);
-    }
-    const nsAString& insertValue = StringTail(
-        aHandlingSetValue.GetSettingValue(), newlength - currentLength);
-
-    if (insertValue.IsEmpty()) {
-      // In this case, we makes the editor stop dispatching "input"
-      // event so that passing nullptr as nsIPrincipal is safe for
-      // now.
-      nsresult rv = textEditor->DeleteSelectionAsAction(
-          nsIEditor::eNone, nsIEditor::eNoStrip, nullptr);
-      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
-                           "TextEditor::DeleteSelectionAsAction() failed");
-      return rv != NS_ERROR_OUT_OF_MEMORY;
-    }
-    // In this case, we makes the editor stop dispatching "input"
-    // event so that passing nullptr as nsIPrincipal is safe for
-    // now.
-    nsresult rv = textEditor->InsertTextAsAction(insertValue, nullptr);
-    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
-                         "TextEditor::InsertTextAsAction() failed");
-    return rv != NS_ERROR_OUT_OF_MEMORY;
-  }
-
   // On <input> or <textarea>, we shouldn't preserve existing undo
   // transactions because other browsers do not preserve them too
   // and not preserving transactions makes setting value faster.
-  AutoDisableUndo disableUndo(textEditor);
+  //
+  // (Except if chrome opts into this behavior).
+  Maybe<AutoDisableUndo> disableUndo;
+  if (!(aHandlingSetValue.GetSetValueFlags() & eSetValue_PreserveHistory)) {
+    disableUndo.emplace(textEditor);
+  }
+
   if (selection) {
     // Since we don't use undo transaction, we don't need to store
     // selection state.  SetText will set selection to tail.

dom/html/TextControlState.h

@@ -194,10 +194,10 @@ class TextControlState final : public SupportsWeakPtr {
     // TODO(mbrodesser): update comment and enumerator identifier to reflect
     // that also the direction is set to forward.
     eSetValue_MoveCursorToEndIfValueChanged = 1 << 4,
-    // The value is changed for a XUL text control as opposed to for an HTML
+
-    // text control.  Such value changes are different in that they preserve the
+    // The value change should preserve undo history.
-    // undo history.
+    eSetValue_PreserveHistory = 1 << 5,
-    eSetValue_ForXUL = 1 << 5,
+
     // Whether it should be tried to move the cursor to the beginning of the
     // text control and set the selection direction to "forward".
     // TODO(mbrodesser): As soon as "none" is supported

layout/base/PresShell.cpp

@@ -2481,12 +2481,6 @@ PresShell::CompleteMove(bool aForward, bool aExtend) {
           nsISelectionController::SCROLL_FOR_CARET_MOVE);
 }
 
-NS_IMETHODIMP
-PresShell::SelectAll() {
-  RefPtr<nsFrameSelection> frameSelection = mSelection;
-  return frameSelection->SelectAll();
-}
-
 static void DoCheckVisibility(nsPresContext* aPresContext, nsIContent* aNode,
                               int16_t aStartOffset, int16_t aEndOffset,
                               bool* aRetval) {

layout/base/PresShell.h

@@ -1320,7 +1320,6 @@ class PresShell final : public nsStubDocumentObserver,
   NS_IMETHOD ScrollCharacter(bool aRight) override;
   NS_IMETHOD CompleteScroll(bool aForward) override;
   NS_IMETHOD CompleteMove(bool aForward, bool aExtend) override;
-  NS_IMETHOD SelectAll() override;
   NS_IMETHOD CheckVisibility(nsINode* node, int16_t startOffset,
                              int16_t EndOffset, bool* _retval) override;
   nsresult CheckVisibilityContent(nsIContent* aNode, int16_t aStartOffset,

layout/generic/nsFrameSelection.cpp

@@ -2129,27 +2129,6 @@ nsFrameSelection::CreateRangeExtendedToSomewhere(
   return range;
 }
 
-nsresult nsFrameSelection::SelectAll() {
-  nsCOMPtr<nsIContent> rootContent;
-  if (mLimiters.mLimiter) {
-    rootContent = mLimiters.mLimiter;  // addrefit
-  } else if (mLimiters.mAncestorLimiter) {
-    rootContent = mLimiters.mAncestorLimiter;
-  } else {
-    NS_ENSURE_STATE(mPresShell);
-    Document* doc = mPresShell->GetDocument();
-    if (!doc) return NS_ERROR_FAILURE;
-    rootContent = doc->GetRootElement();
-    if (!rootContent) return NS_ERROR_FAILURE;
-  }
-  int32_t numChildren = rootContent->GetChildCount();
-  SetChangeReasons(nsISelectionListener::NO_REASON);
-  int8_t index = GetIndexFromSelectionType(SelectionType::eNormal);
-  AutoPrepareFocusRange prep(mDomSelections[index], false);
-  return TakeFocus(rootContent, 0, numChildren, CARET_ASSOCIATE_BEFORE,
-                   FocusMode::kCollapseToNewPoint);
-}
-
 //////////END FRAMESELECTION
 
 void nsFrameSelection::StartBatchChanges() { mBatching.mCounter++; }

layout/generic/nsFrameSelection.h

@@ -608,12 +608,6 @@ class nsFrameSelection final {
                                                      eLogical);
   }
 
-  /**
-   * Select All will generally be called from the nsiselectioncontroller
-   * implementations. it will select the whole doc
-   */
-  MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult SelectAll();
-
   /** Sets/Gets The display selection enum.
    */
   void SetDisplaySelection(int16_t aState) { mDisplaySelection = aState; }