Bug 1343037 part 13. Implement nsTextEditorState::SetSelectionEnd. r=ehsan
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 09 Mar 2017 14:44:05 -0500
changeset 397587 301369c6601ce010962483ac4a72f9b1ea309583
parent 397586 69da5429c4b4ecb2423132f0e33df1c67a55e771
child 397588 2ed58ad7b2d59e3e100858764c884f92b004e3c6
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
bugs1343037
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 1343037 part 13. Implement nsTextEditorState::SetSelectionEnd. 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 selectionEnd changes. 3) Changes the IDL type of HTMLInputElement's selectionEnd attribute to "unsigned long" to match the spec and HTMLTextareaElement. MozReview-Commit-ID: J3Gkhr8VnbS
dom/html/HTMLInputElement.cpp
dom/html/HTMLInputElement.h
dom/html/HTMLTextAreaElement.cpp
dom/html/nsTextEditorState.cpp
dom/html/nsTextEditorState.h
dom/webidl/HTMLInputElement.webidl
testing/web-platform/tests/html/semantics/forms/textfieldselection/selection-start-end.html
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@@ -6454,77 +6454,51 @@ HTMLInputElement::SetSelectionStart(cons
     return;
   }
 
   nsTextEditorState* state = GetEditorState();
   MOZ_ASSERT(state, "SupportsTextSelection() returned true!");
   state->SetSelectionStart(aSelectionStart, aRv);
 }
 
-Nullable<int32_t>
+Nullable<uint32_t>
 HTMLInputElement::GetSelectionEnd(ErrorResult& aRv)
 {
   if (!SupportsTextSelection()) {
-    return Nullable<int32_t>();
-  }
-
-  int32_t selEnd = GetSelectionEndIgnoringType(aRv);
+    return Nullable<uint32_t>();
+  }
+
+  uint32_t selEnd = GetSelectionEndIgnoringType(aRv);
   if (aRv.Failed()) {
-    return Nullable<int32_t>();
-  }
-
-  return Nullable<int32_t>(selEnd);
-}
-
-int32_t
+    return Nullable<uint32_t>();
+  }
+
+  return Nullable<uint32_t>(selEnd);
+}
+
+uint32_t
 HTMLInputElement::GetSelectionEndIgnoringType(ErrorResult& aRv)
 {
   int32_t selEnd, selStart;
   GetSelectionRange(&selStart, &selEnd, aRv);
   return selEnd;
 }
 
 void
-HTMLInputElement::SetSelectionEnd(const Nullable<int32_t>& aSelectionEnd,
+HTMLInputElement::SetSelectionEnd(const Nullable<uint32_t>& aSelectionEnd,
                                   ErrorResult& aRv)
 {
   if (!SupportsTextSelection()) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
-  int32_t selEnd = 0;
-  if (!aSelectionEnd.IsNull()) {
-    selEnd = aSelectionEnd.Value();
-  }
-
   nsTextEditorState* state = GetEditorState();
-  if (state && state->IsSelectionCached()) {
-    state->GetSelectionProperties().SetEnd(selEnd);
-    return;
-  }
-
-  nsAutoString direction;
-  GetSelectionDirection(direction, aRv);
-  if (aRv.Failed()) {
-    return;
-  }
-
-  int32_t start, end;
-  GetSelectionRange(&start, &end, aRv);
-  if (aRv.Failed()) {
-    return;
-  }
-
-  end = selEnd;
-  if (start > end) {
-    start = end;
-  }
-
-  aRv = SetSelectionRange(start, end, direction);
+  MOZ_ASSERT(state, "SupportsTextSelection() returned true!");
+  state->SetSelectionEnd(aSelectionEnd, aRv);
 }
 
 NS_IMETHODIMP
 HTMLInputElement::GetFiles(nsIDOMFileList** aFileList)
 {
   RefPtr<FileList> list = GetFiles();
   list.forget(aFileList);
   return NS_OK;
--- a/dom/html/HTMLInputElement.h
+++ b/dom/html/HTMLInputElement.h
@@ -242,17 +242,17 @@ public:
   NS_IMETHOD_(bool) HasCachedSelection() override;
   virtual void GetSelectionRange(int32_t* aSelectionStart,
                                  int32_t* aSelectionEnd,
                                  ErrorResult& aRv) override;
 
   // Methods for nsFormFillController so it can do selection operations on input
   // types the HTML spec doesn't support them on, like "email".
   uint32_t GetSelectionStartIgnoringType(ErrorResult& aRv);
-  int32_t GetSelectionEndIgnoringType(ErrorResult& aRv);
+  uint32_t GetSelectionEndIgnoringType(ErrorResult& aRv);
 
   void GetDisplayFileName(nsAString& aFileName) const;
 
   const nsTArray<OwningFileOrDirectory>& GetFilesOrDirectoriesInternal() const
   {
     return mFilesOrDirectories;
   }
 
@@ -715,18 +715,18 @@ public:
 
   // XPCOM GetCustomVisibility() is OK
 
   // XPCOM Select() is OK
 
   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);
+  Nullable<uint32_t> GetSelectionEnd(ErrorResult& aRv);
+  void SetSelectionEnd(const Nullable<uint32_t>& aValue, ErrorResult& aRv);
 
   void GetSelectionDirection(nsAString& aValue, ErrorResult& aRv);
   void SetSelectionDirection(const nsAString& aValue, ErrorResult& aRv);
 
   void SetSelectionRange(int32_t aStart, int32_t aEnd,
                          const Optional< nsAString >& direction,
                          ErrorResult& aRv);
 
--- a/dom/html/HTMLTextAreaElement.cpp
+++ b/dom/html/HTMLTextAreaElement.cpp
@@ -720,44 +720,17 @@ HTMLTextAreaElement::GetSelectionEnd(Err
   GetSelectionRange(&selStart, &selEnd, aError);
   return Nullable<uint32_t>(selEnd);
 }
 
 void
 HTMLTextAreaElement::SetSelectionEnd(const Nullable<uint32_t>& aSelectionEnd,
                                      ErrorResult& aError)
 {
-  int32_t selEnd = 0;
-  if (!aSelectionEnd.IsNull()) {
-    selEnd = aSelectionEnd.Value();
-  }
-
-  if (mState.IsSelectionCached()) {
-    mState.GetSelectionProperties().SetEnd(selEnd);
-    return;
-  }
-
-  nsAutoString direction;
-  GetSelectionDirection(direction, aError);
-  if (aError.Failed()) {
-    return;
-  }
-  int32_t start, end;
-  GetSelectionRange(&start, &end, aError);
-  if (aError.Failed()) {
-    return;
-  }
-  end = selEnd;
-  if (start > end) {
-    start = end;
-  }
-  nsresult rv = SetSelectionRange(start, end, direction);
-  if (NS_FAILED(rv)) {
-    aError.Throw(rv);
-  }
+  mState.SetSelectionEnd(aSelectionEnd, aError);
 }
 
 void
 HTMLTextAreaElement::GetSelectionRange(int32_t* aSelectionStart,
                                        int32_t* aSelectionEnd,
                                        ErrorResult& aRv)
 {
   return mState.GetSelectionRange(aSelectionStart, aSelectionEnd, aRv);
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -1732,16 +1732,41 @@ nsTextEditorState::SetSelectionStart(con
 
   if (end < start) {
     end = start;
   }
 
   SetSelectionRange(start, end, dir, aRv);
 }
 
+void
+nsTextEditorState::SetSelectionEnd(const mozilla::dom::Nullable<uint32_t>& aEnd,
+                                   ErrorResult& aRv)
+{
+  int32_t end = 0;
+  if (!aEnd.IsNull()) {
+    // XXXbz This will do the wrong thing for input values that are out of the
+    // int32_t range...
+    end = aEnd.Value();
+  }
+
+  int32_t start, ignored;
+  GetSelectionRange(&start, &ignored, aRv);
+  if (aRv.Failed()) {
+    return;
+  }
+
+  nsITextControlFrame::SelectionDirection dir = GetSelectionDirection(aRv);
+  if (aRv.Failed()) {
+    return;
+  }
+
+  SetSelectionRange(start, end, dir, aRv);
+}
+
 HTMLInputElement*
 nsTextEditorState::GetParentNumberControl(nsFrame* aFrame) const
 {
   MOZ_ASSERT(aFrame);
   nsIContent* content = aFrame->GetContent();
   MOZ_ASSERT(content);
   nsIContent* parent = content->GetParent();
   if (!parent) {
--- a/dom/html/nsTextEditorState.h
+++ b/dom/html/nsTextEditorState.h
@@ -299,16 +299,22 @@ public:
                          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);
 
+  // Set the selection end.  This basically implements the
+  // https://html.spec.whatwg.org/multipage/forms.html#dom-textarea/input-selectionend
+  // setter.
+  void SetSelectionEnd(const mozilla::dom::Nullable<uint32_t>& aEnd,
+                       mozilla::ErrorResult& aRv);
+
   void UpdateEditableState(bool aNotify) {
     if (mRootNode) {
       mRootNode->UpdateEditableState(aNotify);
     }
   }
 
 private:
   friend class RestoreSelectionState;
--- a/dom/webidl/HTMLInputElement.webidl
+++ b/dom/webidl/HTMLInputElement.webidl
@@ -114,17 +114,17 @@ interface HTMLInputElement : HTMLElement
 
   // Bug 850365 readonly attribute NodeList labels;
 
   void select();
 
   [Throws]
            attribute unsigned long? selectionStart;
   [Throws]
-           attribute long? selectionEnd;
+           attribute unsigned long? selectionEnd;
   [Throws]
            attribute DOMString? selectionDirection;
   [Throws]
   void setRangeText(DOMString replacement);
   [Throws]
   void setRangeText(DOMString replacement, unsigned long start,
     unsigned long end, optional SelectionMode selectionMode = "preserve");
 
--- a/testing/web-platform/tests/html/semantics/forms/textfieldselection/selection-start-end.html
+++ b/testing/web-platform/tests/html/semantics/forms/textfieldselection/selection-start-end.html
@@ -50,19 +50,46 @@
         el.remove();
       });
       el.selectionStart = 2;
     }
   }, "onselect should fire when selectionStart is changed");
 
   test(function() {
     for (let el of createTestElements(testValue)) {
+      assert_equals(el.selectionEnd, testValue.length,
+                    `Initial .value set on ${el.id} should set selectionEnd to end of value`);
+      var t = async_test(`onselect should fire when selectionEnd is changed on ${el.id}`);
+      el.onselect = t.step_func_done(function(e) {
+        assert_equals(e.type, "select");
+        el.remove();
+      });
+      el.selectionEnd = 2;
+    }
+  }, "onselect should fire when selectionEnd 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");
+
+  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`);
+      assert_equals(el.selectionEnd, testValue.length,
+                    `Initial .value set on ${el.id} should set selectionEnd to end of value`);
+      el.selectionStart = 8;
+      el.selectionEnd = 5;
+      assert_equals(el.selectionStart, 5, `selectionStart on ${el.id}`);
+      assert_equals(el.selectionEnd, 5, `selectionEnd on ${el.id}`);
+      el.remove();
+    }
+  }, "Setting selectionEnd to a value smaller than selectionStart should decrease selectionStart");
 </script>