Bug 1680548 - Cache the max length in TextControlState::SelectionProperties r=masayuki

GetValue() is not a cheap operation, and this allows to skip it in SetSelectionRange.

Differential Revision: https://phabricator.services.mozilla.com/D98659

dom/html/TextControlState.cpp

@@ -2069,17 +2069,6 @@ void TextControlState::SetSelectionRange(
   bool changed = false;
   nsresult rv = NS_OK;  // For the ScrollSelectionIntoView() return value.
   if (IsSelectionCached()) {
-    nsAutoString value;
-    // XXXbz is "false" the right thing to pass here?  Hard to tell, given the
-    // various mismatches between our impl and the spec.
-    GetValue(value, false);
-    uint32_t length = value.Length();
-    if (aStart > length) {
-      aStart = length;
-    }
-    if (aEnd > length) {
-      aEnd = length;
-    }
     SelectionProperties& props = GetSelectionProperties();
     changed = props.GetStart() != aStart || props.GetEnd() != aEnd ||
               props.GetDirection() != aDirection;
@@ -2893,6 +2882,7 @@ bool TextControlState::SetValueWithoutTextEditor(
             aHandlingSetValue.GetSetValueFlags()));
 
         SelectionProperties& props = GetSelectionProperties();
+        props.SetMaxLength(aHandlingSetValue.GetSettingValue().Length());
         if (aHandlingSetValue.GetSetValueFlags() &
             eSetValue_MoveCursorToEndIfValueChanged) {
           props.SetStart(aHandlingSetValue.GetSettingValue().Length());
@@ -2903,13 +2893,6 @@ bool TextControlState::SetValueWithoutTextEditor(
           props.SetStart(0);
           props.SetEnd(0);
           props.SetDirection(nsITextControlFrame::eForward);
-        } else {
-          // Make sure our cached selection position is not outside the new
-          // value.
-          props.SetStart(std::min(
-              props.GetStart(), aHandlingSetValue.GetSettingValue().Length()));
-          props.SetEnd(std::min(props.GetEnd(),
-                                aHandlingSetValue.GetSettingValue().Length()));
         }
       }
 

dom/html/TextControlState.h

@@ -284,12 +284,12 @@ class TextControlState final : public SupportsWeakPtr {
     uint32_t GetStart() const { return mStart; }
     void SetStart(uint32_t value) {
       mIsDirty = true;
-      mStart = value;
+      mStart = std::min(value, mMaxLength);
     }
     uint32_t GetEnd() const { return mEnd; }
     void SetEnd(uint32_t value) {
       mIsDirty = true;
-      mEnd = value;
+      mEnd = std::min(value, mMaxLength);
     }
     nsITextControlFrame::SelectionDirection GetDirection() const {
       return mDirection;
@@ -298,13 +298,20 @@ class TextControlState final : public SupportsWeakPtr {
       mIsDirty = true;
       mDirection = value;
     }
+    void SetMaxLength(uint32_t aMax) {
+      mMaxLength = aMax;
+      // recompute against the new max length
+      SetStart(GetStart());
+      SetEnd(GetEnd());
+    }
+
     // 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;
+    uint32_t mStart, mEnd, mMaxLength;
     bool mIsDirty = false;
     nsITextControlFrame::SelectionDirection mDirection;
   };

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

@@ -144,6 +144,31 @@
     }
   }, "selectionEnd edge-case values");
 
+  test(() => {
+    for (const el of createTestElements(testValue)) {
+      el.selectionStart = 200;
+      assert_equals(el.selectionStart, testValue.length);
+      el.remove();
+    }
+  }, "selectionStart should be clamped by the current value length");
+
+  test(() => {
+    for (const el of createTestElements(testValue)) {
+      el.selectionStart = 300;
+      assert_equals(el.selectionEnd, testValue.length);
+      el.remove();
+    }
+  }, "selectionEnd should be clamped by the current value length");
+
+  test(() => {
+    for (const el of createTestElements(testValue)) {
+      el.setSelectionRange(200, 300);
+      assert_equals(el.selectionStart, testValue.length);
+      assert_equals(el.selectionEnd, testValue.length);
+      el.remove();
+    }
+  }, "setSelectionRange should be clamped by the current value length");
+
   test(() => {
     for (let el of createTestElements(testValue)) {
       const start = 1;