From 2b6c54e6c15a8762639cc21a9f172382b04b28b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 2 Dec 2020 10:44:25 +0000 Subject: [PATCH] Bug 1676785 - Simplify XUL special code-path for 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 | 4 -- dom/base/test/chrome/chrome.ini | 1 + .../test_input_value_set_preserve_undo.xhtml | 36 ++++++++++ dom/html/HTMLInputElement.cpp | 12 ++-- dom/html/TextControlState.cpp | 69 ++----------------- dom/html/TextControlState.h | 8 +-- layout/base/PresShell.cpp | 6 -- layout/base/PresShell.h | 1 - layout/generic/nsFrameSelection.cpp | 21 ------ layout/generic/nsFrameSelection.h | 6 -- 10 files changed, 56 insertions(+), 108 deletions(-) create mode 100644 dom/base/test/chrome/test_input_value_set_preserve_undo.xhtml diff --git a/dom/base/nsISelectionController.idl b/dom/base/nsISelectionController.idl index 27e76a34207a..2bfa863d31ef 100644 --- a/dom/base/nsISelectionController.idl +++ b/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 diff --git a/dom/base/test/chrome/chrome.ini b/dom/base/test/chrome/chrome.ini index ef307d13b6ad..b342c5a93497 100644 --- a/dom/base/test/chrome/chrome.ini +++ b/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 diff --git a/dom/base/test/chrome/test_input_value_set_preserve_undo.xhtml b/dom/base/test/chrome/test_input_value_set_preserve_undo.xhtml new file mode 100644 index 000000000000..ced5b27927ac --- /dev/null +++ b/dom/base/test/chrome/test_input_value_set_preserve_undo.xhtml @@ -0,0 +1,36 @@ + + + + + + + + + + + + + + + + diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 51d42c79bcee..111f99176783 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/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. - nsIContent* parent = GetParent(); - if (parent && parent->IsXULElement()) { - aFlags |= TextControlState::eSetValue_ForXUL; + // is a XUL node, we consider our control a XUL control. XUL controls preserve + // edit history across value setters. + // + // 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()) { diff --git a/dom/html/TextControlState.cpp b/dom/html/TextControlState.cpp index 3f4c0985b7c7..031e81ce2306 100644 --- a/dom/html/TextControlState.cpp +++ b/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 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 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