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;
@@ -2402,6 +2391,7 @@ void TextControlState::UnbindFromFrame(nsTextControlFrame* aFrame) {
         GetSelectionDirection(IgnoreErrors());
 
     SelectionProperties& props = GetSelectionProperties();
+    props.SetMaxLength(value.Length());
     props.SetStart(start);
     props.SetEnd(end);
     props.SetDirection(direction);
@@ -2902,6 +2892,7 @@ bool TextControlState::SetValueWithoutTextEditor(
             aHandlingSetValue.GetSetValueFlags()));
 
         SelectionProperties& props = GetSelectionProperties();
+        props.SetMaxLength(aHandlingSetValue.GetSettingValue().Length());
         if (aHandlingSetValue.GetSetValueFlags() &
             eSetValue_MoveCursorToEndIfValueChanged) {
           props.SetStart(aHandlingSetValue.GetSettingValue().Length());
@@ -2912,13 +2903,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

@@ -275,8 +275,6 @@ class TextControlState final : public SupportsWeakPtr {
 
   struct SelectionProperties {
    public:
-    SelectionProperties()
-        : mStart(0), mEnd(0), mDirection(nsITextControlFrame::eForward) {}
     bool IsDefault() const {
       return mStart == 0 && mEnd == 0 &&
              mDirection == nsITextControlFrame::eForward;
@@ -284,12 +282,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,15 +296,25 @@ 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 = 0;
+    uint32_t mEnd = 0;
+    uint32_t mMaxLength = 0;
     bool mIsDirty = false;
-    nsITextControlFrame::SelectionDirection mDirection;
+    nsITextControlFrame::SelectionDirection mDirection =
+        nsITextControlFrame::eForward;
   };
 
   bool IsSelectionCached() const { return mSelectionCached; }

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;