Bug 1357206 part 4. Don't move the cursor even if eSetValue_MoveCursorToEndIfValueChanged is set, if the value did not change. r=ehsan
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 01 May 2017 13:28:54 -0400
changeset 406383 c966ab0cfbb7a93c2120f82edee9e5d7fc9b437b
parent 406382 77a461be367e3c02ed9c2296d03da0d9f24a01c6
child 406384 a1d90267f5ed1bb83769f46ad70e2d2aa17fb4ad
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1357206
milestone55.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 1357206 part 4. Don't move the cursor even if eSetValue_MoveCursorToEndIfValueChanged is set, if the value did not change. r=ehsan
devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js
devtools/client/debugger/new/test/mochitest/browser_dbg-expressions.js
devtools/client/debugger/new/test/mochitest/head.js
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
dom/html/nsTextEditorState.cpp
dom/html/nsTextEditorState.h
modules/libpref/init/all.js
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js
@@ -8,16 +8,18 @@ function findBreakpoint(dbg, url, line) 
 }
 
 function setConditionalBreakpoint(dbg, index, condition) {
   return Task.spawn(function* () {
     rightClickElement(dbg, "gutter", index);
     selectMenuItem(dbg, 2);
     yield waitForElement(dbg, ".conditional-breakpoint-panel input");
     findElementWithSelector(dbg, ".conditional-breakpoint-panel input").focus();
+    // Position cursor reliably at the end of the text.
+    pressKey(dbg, "End")
     type(dbg, condition);
     pressKey(dbg, "Enter");
   });
 }
 
 add_task(function* () {
   const dbg = yield initDebugger("doc-scripts.html");
   yield selectSource(dbg, "simple2");
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-expressions.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-expressions.js
@@ -27,16 +27,18 @@ async function addExpression(dbg, input)
   pressKey(dbg, "Enter");
 
   await waitForDispatch(dbg, "EVALUATE_EXPRESSION");
 }
 
 async function editExpression(dbg, input) {
   info("updating the expression");
   dblClickElement(dbg, "expressionNode", 1);
+  // Position cursor reliably at the end of the text.
+  pressKey(dbg, "End")
   type(dbg, input);
   pressKey(dbg, "Enter");
   await waitForDispatch(dbg, "EVALUATE_EXPRESSION");
 }
 
 add_task(function*() {
   const dbg = yield initDebugger("doc-script-switching.html");
 
--- a/devtools/client/debugger/new/test/mochitest/head.js
+++ b/devtools/client/debugger/new/test/mochitest/head.js
@@ -555,16 +555,20 @@ function invokeInTab(fnc) {
 const isLinux = Services.appinfo.OS === "Linux";
 const cmdOrCtrl = isLinux ? { ctrlKey: true } : { metaKey: true };
 const keyMappings = {
   sourceSearch: { code: "p", modifiers: cmdOrCtrl },
   fileSearch: { code: "f", modifiers: cmdOrCtrl },
   Enter: { code: "VK_RETURN" },
   Up: { code: "VK_UP" },
   Down: { code: "VK_DOWN" },
+  Right: { code: "VK_RIGHT" },
+  Left: { code: "VK_LEFT" },
+  End: { code: "VK_RIGHT", modifiers: cmdOrCtrl },
+  Start: { code: "VK_LEFT", modifiers: cmdOrCtrl },
   Tab: { code: "VK_TAB" },
   Escape: { code: "VK_ESCAPE" },
   pauseKey: { code: "VK_F8" },
   resumeKey: { code: "VK_F8" },
   stepOverKey: { code: "VK_F10" },
   stepInKey: { code: "VK_F11", modifiers: { ctrlKey: isLinux } },
   stepOutKey: {
     code: "VK_F11",
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -292,16 +292,17 @@ bool nsContentUtils::sIsExperimentalAuto
 bool nsContentUtils::sIsWebComponentsEnabled = false;
 bool nsContentUtils::sIsCustomElementsEnabled = false;
 bool nsContentUtils::sPrivacyResistFingerprinting = false;
 bool nsContentUtils::sSendPerformanceTimingNotifications = false;
 bool nsContentUtils::sUseActivityCursor = false;
 bool nsContentUtils::sAnimationsAPICoreEnabled = false;
 bool nsContentUtils::sAnimationsAPIElementAnimateEnabled = false;
 bool nsContentUtils::sGetBoxQuadsEnabled = false;
+bool nsContentUtils::sSkipCursorMoveForSameValueSet = false;
 
 int32_t nsContentUtils::sPrivacyMaxInnerWidth = 1000;
 int32_t nsContentUtils::sPrivacyMaxInnerHeight = 1000;
 
 uint32_t nsContentUtils::sHandlingInputTimeout = 1000;
 
 uint32_t nsContentUtils::sCookiesLifetimePolicy = nsICookieService::ACCEPT_NORMALLY;
 uint32_t nsContentUtils::sCookiesBehavior = nsICookieService::BEHAVIOR_ACCEPT;
@@ -640,16 +641,20 @@ nsContentUtils::Init()
                                "dom.animations-api.core.enabled", false);
 
   Preferences::AddBoolVarCache(&sAnimationsAPIElementAnimateEnabled,
                                "dom.animations-api.element-animate.enabled", false);
 
   Preferences::AddBoolVarCache(&sGetBoxQuadsEnabled,
                                "layout.css.getBoxQuads.enabled", false);
 
+  Preferences::AddBoolVarCache(&sSkipCursorMoveForSameValueSet,
+                               "dom.input.skip_cursor_move_for_same_value_set",
+                               true);
+
   Element::InitCCCallbacks();
 
   nsCOMPtr<nsIUUIDGenerator> uuidGenerator =
     do_GetService("@mozilla.org/uuid-generator;1", &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   uuidGenerator.forget(&sUUIDGenerator);
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -2883,16 +2883,23 @@ public:
   static bool
   IsCustomElementsEnabled() { return sIsCustomElementsEnabled; }
 
   /**
    * Compose a tab id with process id and a serial number.
    */
   static uint64_t GenerateTabId();
 
+  /**
+   * Check whether we should skip moving the cursor for a same-value .value set
+   * on a text input or textarea.
+   */
+  static bool
+  SkipCursorMoveForSameValueSet() { return sSkipCursorMoveForSameValueSet; }
+
 private:
   static bool InitializeEventTable();
 
   static nsresult EnsureStringBundle(PropertiesFile aFile);
 
   static bool CanCallerAccess(nsIPrincipal* aSubjectPrincipal,
                                 nsIPrincipal* aPrincipal);
 
@@ -3007,16 +3014,17 @@ private:
   static bool sIsWebComponentsEnabled;
   static bool sIsCustomElementsEnabled;
   static bool sPrivacyResistFingerprinting;
   static bool sSendPerformanceTimingNotifications;
   static bool sUseActivityCursor;
   static bool sAnimationsAPICoreEnabled;
   static bool sAnimationsAPIElementAnimateEnabled;
   static bool sGetBoxQuadsEnabled;
+  static bool sSkipCursorMoveForSameValueSet;
   static uint32_t sCookiesLifetimePolicy;
   static uint32_t sCookiesBehavior;
 
   static int32_t sPrivacyMaxInnerWidth;
   static int32_t sPrivacyMaxInnerHeight;
 
   static nsHtml5StringParser* sHTMLFragmentParser;
   static nsIParser* sXMLFragmentParser;
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -2631,36 +2631,51 @@ nsTextEditorState::SetValue(const nsAStr
         if (selPriv)
           selPriv->EndBatchChanges();
       }
     }
   } else {
     if (!mValue) {
       mValue.emplace();
     }
-    if (!mValue->Assign(newValue, fallible)) {
-      return false;
-    }
-
-    // Since we have no editor we presumably have cached selection state.
-    if (IsSelectionCached()) {
-      SelectionProperties& props = GetSelectionProperties();
-      if (aFlags & eSetValue_MoveCursorToEndIfValueChanged) {
-        props.SetStart(newValue.Length());
-        props.SetEnd(newValue.Length());
-      } else {
-        // Make sure our cached selection position is not outside the new value.
-        props.SetStart(std::min(props.GetStart(), newValue.Length()));
-        props.SetEnd(std::min(props.GetEnd(), newValue.Length()));
+
+    // We can't just early-return here if mValue->Equals(newValue), because
+    // ValueWasChanged and OnValueChanged below still need to be called.
+    if (!mValue->Equals(newValue) ||
+        !nsContentUtils::SkipCursorMoveForSameValueSet()) {
+      if (!mValue->Assign(newValue, fallible)) {
+        return false;
       }
-    }
-
-    // Update the frame display if needed
-    if (mBoundFrame) {
-      mBoundFrame->UpdateValueDisplay(true);
+
+      // Since we have no editor we presumably have cached selection state.
+      if (IsSelectionCached()) {
+        SelectionProperties& props = GetSelectionProperties();
+        if (aFlags & eSetValue_MoveCursorToEndIfValueChanged) {
+          props.SetStart(newValue.Length());
+          props.SetEnd(newValue.Length());
+        } else {
+          // Make sure our cached selection position is not outside the new value.
+          props.SetStart(std::min(props.GetStart(), newValue.Length()));
+          props.SetEnd(std::min(props.GetEnd(), newValue.Length()));
+        }
+      }
+
+      // Update the frame display if needed
+      if (mBoundFrame) {
+        mBoundFrame->UpdateValueDisplay(true);
+      }
+    } else {
+      // Even if our value is not actually changing, apparently we need to mark
+      // our SelectionProperties dirty to make accessibility tests happy.
+      // Probably because they depend on the SetSelectionRange() call we make on
+      // our frame in RestoreSelectionState, but I have no idea why they do.
+      if (IsSelectionCached()) {
+        SelectionProperties& props = GetSelectionProperties();
+        props.SetIsDirty();
+      }
     }
   }
 
   // If we've reached the point where the root node has been created, we
   // can assume that it's safe to notify.
   ValueWasChanged(!!mRootNode);
 
   mTextCtrlElement->OnValueChanged(/* aNotify = */ !!mRootNode,
--- a/dom/html/nsTextEditorState.h
+++ b/dom/html/nsTextEditorState.h
@@ -277,21 +277,26 @@ public:
       {
         return mDirection;
       }
       void SetDirection(nsITextControlFrame::SelectionDirection value)
       {
         mIsDirty = true;
         mDirection = value;
       }
-      // return true only if mStart, mEnd, or mDirection have been modified
+      // return true only if mStart, mEnd, or mDirection have been modified,
+      // or if SetIsDirty() was explicitly called.
       bool IsDirty() const
       {
         return mIsDirty;
       }
+      void SetIsDirty()
+      {
+        mIsDirty = true;
+      }
     private:
       uint32_t mStart, mEnd;
       bool mIsDirty = false;
       nsITextControlFrame::SelectionDirection mDirection;
   };
 
   bool IsSelectionCached() const;
   SelectionProperties& GetSelectionProperties();
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1246,16 +1246,20 @@ pref("dom.forms.select.customstyling", f
 #else
 pref("dom.forms.select.customstyling", true);
 #endif
 pref("dom.select_popup_in_parent.enabled", false);
 
 // Enable Directory API. By default, disabled.
 pref("dom.input.dirpicker", false);
 
+// Enable not moving the cursor to end when a text input or textarea has .value
+// set to the value it already has.  By default, enabled.
+pref("dom.input.skip_cursor_move_for_same_value_set", true);
+
 // Enables system messages and activities
 pref("dom.sysmsg.enabled", false);
 
 // Enable pre-installed applications.
 pref("dom.webapps.useCurrentProfile", false);
 
 pref("dom.cycle_collector.incremental", true);