Bug 821307. Ensure nsTextEditorState::SetValue does nothing when the new value equals the old value, even for password inputs. r=ehsan
authorRobert O'Callahan <robert@ocallahan.org>
Tue, 15 Apr 2014 00:33:47 +1200
changeset 199522 5b54b4287fbf08ae3801de0b42c1c26067852da2
parent 199521 fb7439e1e112b9320dfe7317b133f0a98a072e1c
child 199523 776f4e1090c83c7fe332c929683c0be47b7457a4
push id486
push userasasaki@mozilla.com
push dateMon, 14 Jul 2014 18:39:42 +0000
treeherdermozilla-release@d33428174ff1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs821307
milestone31.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 821307. Ensure nsTextEditorState::SetValue does nothing when the new value equals the old value, even for password inputs. r=ehsan If nsTextEditorState::SetValue is allowed to rebuild the editor DOM even when the new value is the same as the old value, then during PrepareEditor we can remove the content that's the current target of the event triggering the PrepareEditor, which prevents important code from running such as the code that focuses the text input. Normally this isn't a problem but nsTextEditorState::SetValue's code for getting the current value is broken for password controls when !mEditorInitialized. So we remove that broken code path. We have to make sure that the password text, if any, is set on the edit-rules during their initialization so the regular path for getting the current value returns the right thing.
content/html/content/src/nsTextEditorState.cpp
content/html/content/test/mochitest.ini
content/html/content/test/test_bug821307.html
editor/composer/src/nsEditingSession.cpp
editor/idl/nsIEditor.idl
editor/libeditor/base/nsEditRules.h
editor/libeditor/base/nsEditor.cpp
editor/libeditor/html/nsHTMLEditor.cpp
editor/libeditor/html/nsHTMLEditor.h
editor/libeditor/text/nsPlaintextEditor.cpp
editor/libeditor/text/nsPlaintextEditor.h
editor/libeditor/text/nsTextEditRules.cpp
editor/libeditor/text/nsTextEditRules.h
--- a/content/html/content/src/nsTextEditorState.cpp
+++ b/content/html/content/src/nsTextEditorState.cpp
@@ -1229,16 +1229,26 @@ nsTextEditorState::PrepareEditor(const n
       // Set the correct value in the root node
       rv = mBoundFrame->UpdateValueDisplay(true, !mEditorInitialized, aValue);
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     newEditor = mEditor; // just pretend that we have a new editor!
   }
 
+  // Get the current value of the textfield from the content.
+  // Note that if we've created a new editor, mEditor is null at this stage,
+  // so we will get the real value from the content.
+  nsAutoString defaultValue;
+  if (aValue) {
+    defaultValue = *aValue;
+  } else {
+    GetValue(defaultValue, true);
+  }
+
   if (!mEditorInitialized) {
     // Now initialize the editor.
     //
     // NOTE: Conversion of '\n' to <BR> happens inside the
     //       editor's Init() call.
 
     // Get the DOM document
     nsCOMPtr<nsIDOMDocument> domdoc = do_QueryInterface(shell->GetDocument());
@@ -1249,17 +1259,18 @@ nsTextEditorState::PrepareEditor(const n
     // for its content manipulations, and it causes it to fail some security
     // checks deep inside when initializing. So we explictly make it clear that
     // we're native code.
     // Note that any script that's directly trying to access our value
     // has to be going through some scriptable object to do that and that
     // already does the relevant security checks.
     AutoNoJSAPI nojsapi;
 
-    rv = newEditor->Init(domdoc, GetRootNode(), mSelCon, editorFlags);
+    rv = newEditor->Init(domdoc, GetRootNode(), mSelCon, editorFlags,
+                         defaultValue);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // Initialize the controller for the editor
 
   if (!SuppressEventHandlers(presContext)) {
     nsCOMPtr<nsIControllers> controllers;
     nsCOMPtr<nsIDOMHTMLInputElement> inputElement =
@@ -1330,26 +1341,16 @@ nsTextEditorState::PrepareEditor(const n
 
     // Disable the selection if necessary.
     if (editorFlags & nsIPlaintextEditor::eEditorDisabledMask)
       mSelCon->SetDisplaySelection(nsISelectionController::SELECTION_OFF);
 
     newEditor->SetFlags(editorFlags);
   }
 
-  // Get the current value of the textfield from the content.
-  // Note that if we've created a new editor, mEditor is null at this stage,
-  // so we will get the real value from the content.
-  nsAutoString defaultValue;
-  if (aValue) {
-    defaultValue = *aValue;
-  } else {
-    GetValue(defaultValue, true);
-  }
-
   if (shouldInitializeEditor) {
     // Hold on to the newly created editor
     preDestroyer.Swap(mEditor);
   }
 
   // If we have a default value, insert it under the div we created
   // above, but be sure to use the editor so that '*' characters get
   // displayed for password fields, etc. SetValue() will call the
@@ -1775,28 +1776,17 @@ nsTextEditorState::SetValue(const nsAStr
 #ifdef DEBUG
     if (IsSingleLineTextControl()) {
       NS_ASSERTION(mEditorInitialized || mInitializing,
                    "We should never try to use the editor if we're not initialized unless we're being initialized");
     }
 #endif
 
     nsAutoString currentValue;
-    if (!mEditorInitialized && IsSingleLineTextControl()) {
-      // Grab the current value directly from the text node to make sure that we
-      // deal with stale data correctly.
-      NS_ASSERTION(mRootNode, "We should have a root node here");
-      nsIContent *textContent = mRootNode->GetFirstChild();
-      nsCOMPtr<nsIDOMCharacterData> textNode = do_QueryInterface(textContent);
-      if (textNode) {
-        textNode->GetData(currentValue);
-      }
-    } else {
-      mBoundFrame->GetText(currentValue);
-    }
+    mBoundFrame->GetText(currentValue);
 
     nsWeakFrame weakFrame(mBoundFrame);
 
     // this is necessary to avoid infinite recursion
     if (!currentValue.Equals(aValue))
     {
       ValueSetter valueSetter(mEditor);
 
--- a/content/html/content/test/mochitest.ini
+++ b/content/html/content/test/mochitest.ini
@@ -388,16 +388,17 @@ skip-if = (buildapp == 'b2g' && toolkit 
 [test_bug742030.html]
 [test_bug742549.html]
 [test_bug745685.html]
 [test_bug763626.html]
 [test_bug780993.html]
 [test_bug787134.html]
 [test_bug797113.html]
 [test_bug803677.html]
+[test_bug821307.html]
 [test_bug827126.html]
 [test_bug827426.html]
 [test_bug838582.html]
 [test_bug839371.html]
 [test_bug839913.html]
 [test_bug840877.html]
 [test_bug841466.html]
 skip-if = (toolkit == 'gonk' && debug) || e10s #debug-only failure
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug821307.html
@@ -0,0 +1,41 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=821307
+-->
+<head>
+  <title>Test for Bug 821307</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=821307">Mozilla Bug 821307</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+</div>
+
+<input id='dummy'></input>
+<input type="password" id='input' value='11111111111111111' style="width:40em; font-size:40px;"></input>
+
+<pre id="test">
+<script type="application/javascript">
+
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(function() {
+  var dummy = document.getElementById('dummy');
+  dummy.focus();
+  is(document.activeElement, dummy, "Check dummy element is now focused");
+
+  var input = document.getElementById('input');
+  var rect = input.getBoundingClientRect();
+  synthesizeMouse(input, 100, rect.height/2, {});
+  is(document.activeElement, input, "Check input element is now focused");
+
+  SimpleTest.finish();
+});
+
+</script>
+</pre>
+</body>
+</html>
--- a/editor/composer/src/nsEditingSession.cpp
+++ b/editor/composer/src/nsEditingSession.cpp
@@ -443,17 +443,17 @@ nsEditingSession::SetupEditorOnWindow(ns
   NS_ENSURE_TRUE(domDoc, NS_ERROR_FAILURE);
 
   // Set up as a doc state listener
   // Important! We must have this to broadcast the "obs_documentCreated" message
   rv = editor->AddDocumentStateListener(mStateMaintainer);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = editor->Init(domDoc, nullptr /* root content */,
-                    nullptr, mEditorFlags);
+                    nullptr, mEditorFlags, EmptyString());
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsISelection> selection;
   editor->GetSelection(getter_AddRefs(selection));
   nsCOMPtr<nsISelectionPrivate> selPriv = do_QueryInterface(selection);
   NS_ENSURE_TRUE(selPriv, NS_ERROR_FAILURE);
 
   rv = selPriv->AddSelectionListener(mStateMaintainer);
--- a/editor/idl/nsIEditor.idl
+++ b/editor/idl/nsIEditor.idl
@@ -16,17 +16,17 @@ interface nsIDocumentStateListener;
 interface nsIOutputStream;
 interface nsITransactionManager;
 interface nsITransaction;
 interface nsIEditorObserver;
 interface nsIEditActionListener;
 interface nsIInlineSpellChecker;
 interface nsITransferable;
 
-[scriptable, uuid(753b38d1-ee03-4e58-a650-1076ccccdb7f)]
+[scriptable, uuid(65523eab-db1f-44aa-893e-dfe57ad306f0)]
 
 interface nsIEditor  : nsISupports
 {
 %{C++
   typedef short EDirection;
   typedef short EStripWrappers;
 %}
   const short eNone = 0;
@@ -51,17 +51,18 @@ interface nsIEditor  : nsISupports
    * @param aSelCon       this should be used to get the selection location
    *                      (will be null for HTML editors)
    * @param aFlags        A bitmask of flags for specifying the behavior
    *                      of the editor.
    */
   [noscript] void init(in nsIDOMDocument doc,
                        in nsIContent aRoot,
                        in nsISelectionController aSelCon,
-                       in unsigned long aFlags);
+                       in unsigned long aFlags,
+                       in AString initialValue);
 
   void setAttributeOrEquivalent(in nsIDOMElement element,
                                 in AString sourceAttrName,
                                 in AString sourceAttrValue,
                                 in boolean aSuppressTransaction);
   void removeAttributeOrEquivalent(in nsIDOMElement element,
                                    in DOMString sourceAttrName,
                                    in boolean aSuppressTransaction);
--- a/editor/libeditor/base/nsEditRules.h
+++ b/editor/libeditor/base/nsEditRules.h
@@ -1,20 +1,19 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsEditRules_h__
 #define nsEditRules_h__
 
-// FB45AC36-E8F1-44ae-8FB7-466E1BE119B0
 #define NS_IEDITRULES_IID \
-{ 0x2cc50d11, 0x9909, 0x433f, \
-  { 0xb6, 0xfb, 0x4c, 0xf2, 0x56, 0xe5, 0xe5, 0x71 } }
+{ 0x3836386d, 0x806a, 0x488d, \
+  { 0x8b, 0xab, 0xaf, 0x42, 0xbb, 0x4c, 0x90, 0x66 } }
 
 #include "nsEditor.h"
 
 class nsPlaintextEditor;
 class nsISelection;
 
 /***************************************************************************
  * base for an object to encapsulate any additional info needed to be passed
@@ -38,16 +37,17 @@ class nsIEditRules : public nsISupports
 {
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IEDITRULES_IID)
   
 //Interfaces for addref and release and queryinterface
 //NOTE: Use   NS_DECL_ISUPPORTS_INHERITED in any class inherited from nsIEditRules
 
   NS_IMETHOD Init(nsPlaintextEditor *aEditor)=0;
+  NS_IMETHOD SetInitialValue(const nsAString& aValue) = 0;
   NS_IMETHOD DetachEditor()=0;
   NS_IMETHOD BeforeEdit(EditAction action,
                         nsIEditor::EDirection aDirection) = 0;
   NS_IMETHOD AfterEdit(EditAction action,
                        nsIEditor::EDirection aDirection) = 0;
   NS_IMETHOD WillDoAction(mozilla::Selection* aSelection, nsRulesInfo* aInfo,
                           bool* aCancel, bool* aHandled) = 0;
   NS_IMETHOD DidDoAction(nsISelection *aSelection, nsRulesInfo *aInfo, nsresult aResult)=0;
--- a/editor/libeditor/base/nsEditor.cpp
+++ b/editor/libeditor/base/nsEditor.cpp
@@ -199,17 +199,19 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIEditor)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsEditor)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsEditor)
 
 
 NS_IMETHODIMP
-nsEditor::Init(nsIDOMDocument *aDoc, nsIContent *aRoot, nsISelectionController *aSelCon, uint32_t aFlags)
+nsEditor::Init(nsIDOMDocument *aDoc, nsIContent *aRoot,
+               nsISelectionController *aSelCon, uint32_t aFlags,
+               const nsAString& aValue)
 {
   NS_PRECONDITION(aDoc, "bad arg");
   if (!aDoc)
     return NS_ERROR_NULL_POINTER;
 
   // First only set flags, but other stuff shouldn't be initialized now.
   // Don't move this call after initializing mDocWeak.
   // SetFlags() can check whether it's called during initialization or not by
--- a/editor/libeditor/html/nsHTMLEditor.cpp
+++ b/editor/libeditor/html/nsHTMLEditor.cpp
@@ -220,30 +220,32 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_
   NS_INTERFACE_MAP_ENTRY(nsIMutationObserver)
 NS_INTERFACE_MAP_END_INHERITING(nsPlaintextEditor)
 
 
 NS_IMETHODIMP
 nsHTMLEditor::Init(nsIDOMDocument *aDoc,
                    nsIContent *aRoot,
                    nsISelectionController *aSelCon,
-                   uint32_t aFlags)
+                   uint32_t aFlags,
+                   const nsAString& aInitialValue)
 {
   NS_PRECONDITION(aDoc && !aSelCon, "bad arg");
   NS_ENSURE_TRUE(aDoc, NS_ERROR_NULL_POINTER);
+  MOZ_ASSERT(aInitialValue.IsEmpty(), "Non-empty initial values not supported");
 
   nsresult result = NS_OK, rulesRes = NS_OK;
    
   if (1)
   {
     // block to scope nsAutoEditInitRulesTrigger
     nsAutoEditInitRulesTrigger rulesTrigger(static_cast<nsPlaintextEditor*>(this), rulesRes);
 
     // Init the plaintext editor
-    result = nsPlaintextEditor::Init(aDoc, aRoot, nullptr, aFlags);
+    result = nsPlaintextEditor::Init(aDoc, aRoot, nullptr, aFlags, aInitialValue);
     if (NS_FAILED(result)) { return result; }
 
     // Init mutation observer
     nsCOMPtr<nsINode> document = do_QueryInterface(aDoc);
     document->AddMutationObserverUnlessExists(this);
 
     // disable Composer-only features
     if (IsMailEditor())
--- a/editor/libeditor/html/nsHTMLEditor.h
+++ b/editor/libeditor/html/nsHTMLEditor.h
@@ -244,17 +244,19 @@ public:
                                   nsIContent** outNode = nullptr,
                                   int32_t* outOffset = 0);
 
   /* ------------ Overrides of nsEditor interface methods -------------- */
 
   nsresult EndUpdateViewBatch();
 
   /** prepare the editor for use */
-  NS_IMETHOD Init(nsIDOMDocument *aDoc, nsIContent *aRoot, nsISelectionController *aSelCon, uint32_t aFlags);
+  NS_IMETHOD Init(nsIDOMDocument *aDoc, nsIContent *aRoot,
+                  nsISelectionController *aSelCon, uint32_t aFlags,
+                  const nsAString& aValue);
   NS_IMETHOD PreDestroy(bool aDestroyingFrames);
 
   /** Internal, static version */
   // aElement must not be null.
   static bool NodeIsBlockStatic(const mozilla::dom::Element* aElement);
   static nsresult NodeIsBlockStatic(nsIDOMNode *aNode, bool *aIsBlock);
 protected:
   using nsEditor::IsBlockNode;
--- a/editor/libeditor/text/nsPlaintextEditor.cpp
+++ b/editor/libeditor/text/nsPlaintextEditor.cpp
@@ -110,41 +110,48 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_
   NS_INTERFACE_MAP_ENTRY(nsIPlaintextEditor)
   NS_INTERFACE_MAP_ENTRY(nsIEditorMailSupport)
 NS_INTERFACE_MAP_END_INHERITING(nsEditor)
 
 
 NS_IMETHODIMP nsPlaintextEditor::Init(nsIDOMDocument *aDoc, 
                                       nsIContent *aRoot,
                                       nsISelectionController *aSelCon,
-                                      uint32_t aFlags)
+                                      uint32_t aFlags,
+                                      const nsAString& aInitialValue)
 {
   NS_PRECONDITION(aDoc, "bad arg");
   NS_ENSURE_TRUE(aDoc, NS_ERROR_NULL_POINTER);
   
   nsresult res = NS_OK, rulesRes = NS_OK;
   if (mRules) {
     mRules->DetachEditor();
     mRules = nullptr;
   }
   
-  if (1)
   {
     // block to scope nsAutoEditInitRulesTrigger
     nsAutoEditInitRulesTrigger rulesTrigger(this, rulesRes);
   
     // Init the base editor
-    res = nsEditor::Init(aDoc, aRoot, aSelCon, aFlags);
+    res = nsEditor::Init(aDoc, aRoot, aSelCon, aFlags, aInitialValue);
   }
 
   // check the "single line editor newline handling"
   // and "caret behaviour in selection" prefs
   GetDefaultEditorPrefs(mNewlineHandling, mCaretStyle);
 
   NS_ENSURE_SUCCESS(rulesRes, rulesRes);
+
+  // mRules may not have been initialized yet, when this is called via
+  // nsHTMLEditor::Init.
+  if (mRules) {
+    mRules->SetInitialValue(aInitialValue);
+  }
+
   return res;
 }
 
 static int32_t sNewlineHandlingPref = -1,
                sCaretStylePref = -1;
 
 static void
 EditorPrefsChangedCallback(const char *aPrefName, void *)
--- a/editor/libeditor/text/nsPlaintextEditor.h
+++ b/editor/libeditor/text/nsPlaintextEditor.h
@@ -68,17 +68,19 @@ public:
                                       const nsAString & aAttribute,
                                       const nsAString & aValue,
                                       bool aSuppressTransaction);
   NS_IMETHOD RemoveAttributeOrEquivalent(nsIDOMElement * aElement,
                                          const nsAString & aAttribute,
                                          bool aSuppressTransaction);
 
   /** prepare the editor for use */
-  NS_IMETHOD Init(nsIDOMDocument *aDoc, nsIContent *aRoot, nsISelectionController *aSelCon, uint32_t aFlags);
+  NS_IMETHOD Init(nsIDOMDocument *aDoc, nsIContent *aRoot,
+                  nsISelectionController *aSelCon, uint32_t aFlags,
+                  const nsAString& aValue);
   
   NS_IMETHOD GetDocumentIsEmpty(bool *aDocumentIsEmpty);
   NS_IMETHOD GetIsDocumentEditable(bool *aIsDocumentEditable);
 
   NS_IMETHOD DeleteSelection(EDirection aAction,
                              EStripWrappers aStripWrappers);
 
   NS_IMETHOD SetDocumentCharacterSet(const nsACString & characterSet);
--- a/editor/libeditor/text/nsTextEditRules.cpp
+++ b/editor/libeditor/text/nsTextEditRules.cpp
@@ -131,16 +131,25 @@ nsTextEditRules::Init(nsPlaintextEditor 
 
   mDeleteBidiImmediately =
     Preferences::GetBool("bidi.edit.delete_immediately", false);
 
   return res;
 }
 
 NS_IMETHODIMP
+nsTextEditRules::SetInitialValue(const nsAString& aValue)
+{
+  if (IsPasswordEditor()) {
+    mPasswordText = aValue;
+  }
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 nsTextEditRules::DetachEditor()
 {
   if (mTimer)
     mTimer->Cancel();
 
   mEditor = nullptr;
   return NS_OK;
 }
--- a/editor/libeditor/text/nsTextEditRules.h
+++ b/editor/libeditor/text/nsTextEditRules.h
@@ -42,16 +42,17 @@ public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsTextEditRules, nsIEditRules)
   
               nsTextEditRules();
   virtual     ~nsTextEditRules();
 
   // nsIEditRules methods
   NS_IMETHOD Init(nsPlaintextEditor *aEditor);
+  NS_IMETHOD SetInitialValue(const nsAString& aValue);
   NS_IMETHOD DetachEditor();
   NS_IMETHOD BeforeEdit(EditAction action,
                         nsIEditor::EDirection aDirection);
   NS_IMETHOD AfterEdit(EditAction action,
                        nsIEditor::EDirection aDirection);
   NS_IMETHOD WillDoAction(mozilla::Selection* aSelection, nsRulesInfo* aInfo,
                           bool* aCancel, bool* aHandled);
   NS_IMETHOD DidDoAction(nsISelection *aSelection, nsRulesInfo *aInfo, nsresult aResult);