Bug 1343037 part 12. Implement nsTextEditorState::SetSelectionStart. r=ehsan

This introduces three behavior changes:

1)  Before this change, in cached mode, we did not enforce the "start <= end"
    invariant.
2)  Before this change, in cached mode, we did not fire "select" events on
    selectionStart changes.
3)  Changes the IDL type of HTMLInputElement's selectionStart attribute to
    "unsigned long" to match the spec and HTMLTextareaElement.

MozReview-Commit-ID: JM9XXMMPUHM

dom/html/HTMLInputElement.cpp

@@ -6422,22 +6422,22 @@ HTMLInputElement::SetRangeText(const nsAString& aReplacement, uint32_t aStart,
   SetSelectionRange(aSelectionStart, aSelectionEnd, direction, aRv);
 }
 
-Nullable<int32_t>
+Nullable<uint32_t>
 HTMLInputElement::GetSelectionStart(ErrorResult& aRv)
 {
   if (!SupportsTextSelection()) {
-    return Nullable<int32_t>();
+    return Nullable<uint32_t>();
   }
 
-  int32_t selStart = GetSelectionStartIgnoringType(aRv);
+  uint32_t selStart = GetSelectionStartIgnoringType(aRv);
   if (aRv.Failed()) {
-    return Nullable<int32_t>();
+    return Nullable<uint32_t>();
   }
 
-  return Nullable<int32_t>(selStart);
+  return Nullable<uint32_t>(selStart);
 }
 
-int32_t
+uint32_t
 HTMLInputElement::GetSelectionStartIgnoringType(ErrorResult& aRv)
 {
   int32_t selEnd, selStart;
@@ -6446,7 +6446,7 @@ HTMLInputElement::GetSelectionStartIgnoringType(ErrorResult& aRv)
 }
 
 void
-HTMLInputElement::SetSelectionStart(const Nullable<int32_t>& aSelectionStart,
+HTMLInputElement::SetSelectionStart(const Nullable<uint32_t>& aSelectionStart,
                                     ErrorResult& aRv)
 {
   if (!SupportsTextSelection()) {
@@ -6454,35 +6454,9 @@ HTMLInputElement::SetSelectionStart(const Nullable<int32_t>& aSelectionStart,
     return;
   }
 
-  int32_t selStart = 0;
-  if (!aSelectionStart.IsNull()) {
-    selStart = aSelectionStart.Value();
-  }
-
   nsTextEditorState* state = GetEditorState();
-  if (state && state->IsSelectionCached()) {
-    state->GetSelectionProperties().SetStart(selStart);
-    return;
-  }
-
-  nsAutoString direction;
-  GetSelectionDirection(direction, aRv);
-  if (aRv.Failed()) {
-    return;
-  }
-
-  int32_t start, end;
-  GetSelectionRange(&start, &end, aRv);
-  if (aRv.Failed()) {
-    return;
-  }
-
-  start = selStart;
-  if (end < start) {
-    end = start;
-  }
-
-  aRv = SetSelectionRange(start, end, direction);
+  MOZ_ASSERT(state, "SupportsTextSelection() returned true!");
+  state->SetSelectionStart(aSelectionStart, aRv);
 }
 
 Nullable<int32_t>

dom/html/HTMLInputElement.h

@@ -246,7 +246,7 @@ public:
 
   // Methods for nsFormFillController so it can do selection operations on input
   // types the HTML spec doesn't support them on, like "email".
-  int32_t GetSelectionStartIgnoringType(ErrorResult& aRv);
+  uint32_t GetSelectionStartIgnoringType(ErrorResult& aRv);
   int32_t GetSelectionEndIgnoringType(ErrorResult& aRv);
 
   void GetDisplayFileName(nsAString& aFileName) const;
@@ -717,8 +717,8 @@ public:
 
   // XPCOM Select() is OK
 
-  Nullable<int32_t> GetSelectionStart(ErrorResult& aRv);
-  void SetSelectionStart(const Nullable<int32_t>& aValue, ErrorResult& aRv);
+  Nullable<uint32_t> GetSelectionStart(ErrorResult& aRv);
+  void SetSelectionStart(const Nullable<uint32_t>& aValue, ErrorResult& aRv);
 
   Nullable<int32_t> GetSelectionEnd(ErrorResult& aRv);
   void SetSelectionEnd(const Nullable<int32_t>& aValue, ErrorResult& aRv);

dom/html/HTMLTextAreaElement.cpp

@@ -710,34 +710,7 @@ void
 HTMLTextAreaElement::SetSelectionStart(const Nullable<uint32_t>& aSelectionStart,
                                        ErrorResult& aError)
 {
-  int32_t selStart = 0;
-  if (!aSelectionStart.IsNull()) {
-    selStart = aSelectionStart.Value();
-  }
-
-  if (mState.IsSelectionCached()) {
-    mState.GetSelectionProperties().SetStart(selStart);
-    return;
-  }
-
-  nsAutoString direction;
-  GetSelectionDirection(direction, aError);
-  if (aError.Failed()) {
-    return;
-  }
-  int32_t start, end;
-  GetSelectionRange(&start, &end, aError);
-  if (aError.Failed()) {
-    return;
-  }
-  start = selStart;
-  if (end < start) {
-    end = start;
-  }
-  nsresult rv = SetSelectionRange(start, end, direction);
-  if (NS_FAILED(rv)) {
-    aError.Throw(rv);
-  }
+  mState.SetSelectionStart(aSelectionStart, aError);
 }
 
 Nullable<uint32_t>

dom/html/nsTextEditorState.cpp

@@ -1708,6 +1708,35 @@ nsTextEditorState::SetSelectionRange(int32_t aStart, int32_t aEnd,
   }
 }
 
+void
+nsTextEditorState::SetSelectionStart(const mozilla::dom::Nullable<uint32_t>& aStart,
+                                     ErrorResult& aRv)
+{
+  int32_t start = 0;
+  if (!aStart.IsNull()) {
+    // XXXbz This will do the wrong thing for input values that are out of the
+    // int32_t range...
+    start = aStart.Value();
+  }
+
+  int32_t ignored, end;
+  GetSelectionRange(&ignored, &end, aRv);
+  if (aRv.Failed()) {
+    return;
+  }
+
+  nsITextControlFrame::SelectionDirection dir = GetSelectionDirection(aRv);
+  if (aRv.Failed()) {
+    return;
+  }
+
+  if (end < start) {
+    end = start;
+  }
+
+  SetSelectionRange(start, end, dir, aRv);
+}
+
 HTMLInputElement*
 nsTextEditorState::GetParentNumberControl(nsFrame* aFrame) const
 {

dom/html/nsTextEditorState.h

@@ -15,6 +15,7 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/WeakPtr.h"
+#include "mozilla/dom/Nullable.h"
 
 class nsTextInputListener;
 class nsTextControlFrame;
@@ -297,6 +298,12 @@ public:
                          nsITextControlFrame::SelectionDirection aDirection,
                          mozilla::ErrorResult& aRv);
 
+  // Set the selection start.  This basically implements the
+  // https://html.spec.whatwg.org/multipage/forms.html#dom-textarea/input-selectionstart
+  // setter.
+  void SetSelectionStart(const mozilla::dom::Nullable<uint32_t>& aStart,
+                         mozilla::ErrorResult& aRv);
+
   void UpdateEditableState(bool aNotify) {
     if (mRootNode) {
       mRootNode->UpdateEditableState(aNotify);

dom/webidl/HTMLInputElement.webidl

@@ -117,8 +117,7 @@ interface HTMLInputElement : HTMLElement {
   void select();
 
   [Throws]
-           // TODO: unsigned vs signed
-           attribute long? selectionStart;
+           attribute unsigned long? selectionStart;
   [Throws]
            attribute long? selectionEnd;
   [Throws]

testing/web-platform/meta/MANIFEST.json

@@ -93542,6 +93542,12 @@
      {}
     ]
    ],
+   "html/semantics/forms/textfieldselection/selection-start-end.html": [
+    [
+     "/html/semantics/forms/textfieldselection/selection-start-end.html",
+     {}
+    ]
+   ],
    "html/semantics/forms/textfieldselection/selection.html": [
     [
      "/html/semantics/forms/textfieldselection/selection.html",
@@ -176663,12 +176669,16 @@
    "39ecb031aca3655a06152f94a514981fe59ebbaf",
    "testharness"
   ],
+  "html/semantics/forms/textfieldselection/selection-start-end.html": [
+   "1f3184b72aba5631d6db4379dfa98035ee047283",
+   "testharness"
+  ],
   "html/semantics/forms/textfieldselection/selection.html": [
-   "d869799718137671a2eacc323aa26ea4364e845f",
+   "f7674721b84ec8fca0e5e40258447ce857b87784",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/textfieldselection-setRangeText.html": [
-   "0caf2b08ccc5a35578291af8f5adaf7de9537d66",
+   "3bbd350321f5ec9e0a8f3d47da4e11aaa3ad4d68",
    "testharness"
   ],
   "html/semantics/forms/textfieldselection/textfieldselection-setSelectionRange.html": [

testing/web-platform/tests/html/semantics/forms/textfieldselection/selection-start-end.html

@@ -0,0 +1,68 @@
+<!doctype html>
+<meta charset=utf-8>
+<title></title>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<div id=log></div>
+<script>
+  function createInputElement(value, append) {
+    var el = document.createElement("input");
+    el.type = "text";
+    el.value = value;
+    el.id = "input" + (append ? "-appended" : "-not-appended");
+    if (append) {
+      document.body.appendChild(el);
+    }
+    return el;
+  };
+
+  function createTextareaElement(value, append) {
+    var el = document.createElement("textarea");
+    el.value = value;
+
+    el.id = "textarea" + (append ? "-appended" : "-not-appended");
+    if (append) {
+      document.body.appendChild(el);
+    }
+    return el;
+  };
+
+  function createTestElements(value) {
+    return [ createInputElement(value, true),
+             createInputElement(value, false),
+             createTextareaElement(value, true),
+             createTextareaElement(value, false) ];
+  }
+
+  const testValue = "abcdefghij";
+
+  test(function() {
+    assert_equals(testValue.length, 10);
+  }, "Sanity check for testValue length; if this fails, variou absolute offsets in the test below need to be adjusted to be less than testValue.length");
+
+  test(function() {
+    for (let el of createTestElements(testValue)) {
+      assert_equals(el.selectionStart, testValue.length,
+                    `Initial .value set on ${el.id} should set selectionStart to end of value`);
+      var t = async_test(`onselect should fire when selectionStart is changed on ${el.id}`);
+      el.onselect = t.step_func_done(function(e) {
+        assert_equals(e.type, "select");
+        el.remove();
+      });
+      el.selectionStart = 2;
+    }
+  }, "onselect should fire when selectionStart is changed");
+
+  test(function() {
+    for (let el of createTestElements(testValue)) {
+      assert_equals(el.selectionStart, testValue.length,
+                    `Initial .value set on ${el.id} should set selectionStart to end of value`);
+      el.selectionStart = 0;
+      el.selectionEnd = 5;
+      el.selectionStart = 8;
+      assert_equals(el.selectionStart, 8, `selectionStart on ${el.id}`);
+      assert_equals(el.selectionEnd, 8, `selectionEnd on ${el.id}`);
+      el.remove();
+    }
+  }, "Setting selectionStart to a value larger than selectionEnd should increase selectionEnd");
+</script>

testing/web-platform/tests/html/semantics/forms/textfieldselection/textfieldselection-setRangeText.html

@@ -106,6 +106,13 @@
     }, element.id + " setRangeText without argument throws a type error");
 
     async_test(function() {
+      // At this point there are already "select" events queued up on
+      // "element".  Give them time to fire; otherwise we can get spurious
+      // passes.
+      //
+      // This is unfortunately racy in that we might _still_ get spurious
+      // passes.  I'm not sure how best to handle that.
+      setTimeout(this.step_func(function() {
         var q = false;
         element.onselect = this.step_func_done(function(e) {
           assert_true(q, "event should be queued");
@@ -115,6 +122,7 @@
         });
         element.setRangeText("foobar2", 0, 6);
         q = true;
+      }), 10);
     }, element.id + " setRangeText fires a select event");
   })
 </script>