Bug 1893532 - Improve error message of HTMLInputElement.stepUp/stepDown. r=dom-core,sefeng

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

dom/html/HTMLInputElement.cpp

@@ -1986,18 +1986,21 @@ Decimal HTMLInputElement::GetStepBase() const {
   return kDefaultStepBase;
 }
 
-nsresult HTMLInputElement::GetValueIfStepped(int32_t aStep,
-                                             StepCallerType aCallerType,
-                                             Decimal* aNextStep) {
+Decimal HTMLInputElement::GetValueIfStepped(int32_t aStep,
+                                            StepCallerType aCallerType,
+                                            ErrorResult& aRv) {
+  constexpr auto kNaN = Decimal::nan();
   if (!DoStepDownStepUpApply()) {
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
+    aRv.ThrowInvalidStateError("Step doesn't apply to this input type");
+    return kNaN;
   }
 
   Decimal stepBase = GetStepBase();
   Decimal step = GetStep();
   if (step == kStepAny) {
-    if (aCallerType != CALLED_FOR_USER_EVENT) {
-      return NS_ERROR_DOM_INVALID_STATE_ERR;
+    if (aCallerType != StepCallerType::ForUserEvent) {
+      aRv.ThrowInvalidStateError("Can't step an input with step=\"any\"");
+      return kNaN;
     }
     // Allow the spin buttons and up/down arrow keys to do something sensible:
     step = GetDefaultStep();
@@ -2015,7 +2018,7 @@ nsresult HTMLInputElement::GetValueIfStepped(int32_t aStep,
         // adjustment to align maximum on a step, or else (if we adjusted
         // maximum) there is no valid step between minimum and the unadjusted
         // maximum.
-        return NS_OK;
+        return kNaN;
       }
     }
   }
@@ -2062,25 +2065,20 @@ nsresult HTMLInputElement::GetValueIfStepped(int32_t aStep,
        (aStep < 0 && value > valueBeforeStepping))) {
     // We don't want step-up to effectively step down, or step-down to
     // effectively step up, so return;
-    return NS_OK;
+    return kNaN;
   }
 
-  *aNextStep = value;
-  return NS_OK;
+  return value;
 }
 
-nsresult HTMLInputElement::ApplyStep(int32_t aStep) {
-  Decimal nextStep = Decimal::nan();  // unchanged if value will not change
-
-  nsresult rv = GetValueIfStepped(aStep, CALLED_FOR_SCRIPT, &nextStep);
-
-  if (NS_SUCCEEDED(rv) && nextStep.isFinite()) {
-    // We know we're not a file input, so the caller type does not matter; just
-    // pass "not system" to be safe.
-    SetValue(nextStep, CallerType::NonSystem);
+void HTMLInputElement::ApplyStep(int32_t aStep, ErrorResult& aRv) {
+  Decimal nextStep = GetValueIfStepped(aStep, StepCallerType::ForScript, aRv);
+  if (aRv.Failed() || !nextStep.isFinite()) {
+    return;
   }
-
-  return rv;
+  // We know we're not a file input, so the caller type does not matter; just
+  // pass "not system" to be safe.
+  SetValue(nextStep, CallerType::NonSystem);
 }
 
 bool HTMLInputElement::IsDateTimeInputType(FormControlType aType) {
@@ -3520,11 +3518,9 @@ void HTMLInputElement::StepNumberControlForUserEvent(int32_t aDirection) {
     }
   }
 
-  Decimal newValue = Decimal::nan();  // unchanged if value will not change
-
-  nsresult rv = GetValueIfStepped(aDirection, CALLED_FOR_USER_EVENT, &newValue);
-
-  if (NS_FAILED(rv) || !newValue.isFinite()) {
+  Decimal newValue = GetValueIfStepped(aDirection, StepCallerType::ForUserEvent,
+                                       IgnoreErrors());
+  if (!newValue.isFinite()) {
     return;  // value should not or will not change
   }
 

dom/html/HTMLInputElement.h

@@ -663,9 +663,8 @@ class HTMLInputElement final : public TextControlElement,
     SetUnsignedIntAttr(nsGkAtoms::width, aValue, 0, aRv);
   }
 
-  void StepUp(int32_t aN, ErrorResult& aRv) { aRv = ApplyStep(aN); }
-
-  void StepDown(int32_t aN, ErrorResult& aRv) { aRv = ApplyStep(-aN); }
+  void StepUp(int32_t aN, ErrorResult& aRv) { ApplyStep(aN, aRv); }
+  void StepDown(int32_t aN, ErrorResult& aRv) { ApplyStep(-aN, aRv); }
 
   /**
    * Returns the current step value.
@@ -1311,22 +1310,17 @@ class HTMLInputElement final : public TextControlElement,
    */
   Decimal GetDefaultStep() const;
 
-  enum StepCallerType { CALLED_FOR_USER_EVENT, CALLED_FOR_SCRIPT };
+  enum class StepCallerType { ForUserEvent, ForScript };
 
   /**
-   * Sets the aValue outparam to the value that this input would take if
-   * someone tries to step aStep steps and this input's value would change as
-   * a result. Leaves aValue untouched if this inputs value would not change
-   * (e.g. already at max, and asking for the next step up).
+   * Returns the value that this input would take if someone tries to step
+   * aStepCount steps and this input's value would change as a result, or
+   * Decimal::nan() otherwise (e.g., if this inputs value would not change due
+   * to it being already at max, and asking for the next step up).
    *
    * Negative aStep means step down, positive means step up.
-   *
-   * Returns NS_OK or else the error values that should be thrown if this call
-   * was initiated by a stepUp()/stepDown() call from script under conditions
-   * that such a call should throw.
    */
-  nsresult GetValueIfStepped(int32_t aStepCount, StepCallerType aCallerType,
-                             Decimal* aNextStep);
+  Decimal GetValueIfStepped(int32_t aStepCount, StepCallerType, ErrorResult&);
 
   /**
    * Apply a step change from stepUp or stepDown by multiplying aStep by the
@@ -1334,7 +1328,7 @@ class HTMLInputElement final : public TextControlElement,
    *
    * @param aStep The value used to be multiplied against the step value.
    */
-  nsresult ApplyStep(int32_t aStep);
+  void ApplyStep(int32_t aStep, ErrorResult&);
 
   /**
    * Returns if the current type is an experimental mobile type.

mozglue/misc/decimal/Decimal.cpp

@@ -239,11 +239,6 @@ Decimal::Decimal(int32_t i32)
 Decimal::Decimal(Sign sign, int exponent, uint64_t coefficient)
     : m_data(sign, coefficient ? exponent : 0, coefficient) {}
 
-Decimal::Decimal(const EncodedData& data)
-    : m_data(data)
-{
-}
-
 Decimal::Decimal(const Decimal& other)
     : m_data(other.m_data)
 {
@@ -853,11 +848,6 @@ Decimal Decimal::infinity(const Sign sign)
     return Decimal(EncodedData(sign, EncodedData::ClassInfinity));
 }
 
-Decimal Decimal::nan()
-{
-    return Decimal(EncodedData(Positive, EncodedData::ClassNaN));
-}
-
 Decimal Decimal::remainder(const Decimal& rhs) const
 {
     const Decimal quotient = *this / rhs;

mozglue/misc/decimal/Decimal.h

@@ -234,11 +234,13 @@ public:
     // Note: fromString doesn't support "infinity" and "nan".
     static MFBT_API Decimal fromString(const std::string& aValue);
     static MFBT_API Decimal infinity(Sign);
-    static MFBT_API Decimal nan();
+    static constexpr Decimal nan() {
+        return Decimal(EncodedData(Positive, EncodedData::ClassNaN));
+    }
     static MFBT_API Decimal zero(Sign);
 
     // You should not use below methods. We expose them for unit testing.
-    MFBT_API explicit Decimal(const EncodedData&);
+    constexpr explicit Decimal(const EncodedData& data) : m_data(data) {}
     const EncodedData& value() const { return m_data; }
 
 private: