From 50eb344091d103c7f133268c6cdd7a1b5a9e04b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 1 May 2024 00:11:33 +0000 Subject: [PATCH] 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 | 48 +++++++++++++++----------------- dom/html/HTMLInputElement.h | 24 ++++++---------- mozglue/misc/decimal/Decimal.cpp | 10 ------- mozglue/misc/decimal/Decimal.h | 6 ++-- 4 files changed, 35 insertions(+), 53 deletions(-) diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 958601616b8f..742db28bf27a 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/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 } diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h index 2c90aa83fe8b..4c54e9024179 100644 --- a/dom/html/HTMLInputElement.h +++ b/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. diff --git a/mozglue/misc/decimal/Decimal.cpp b/mozglue/misc/decimal/Decimal.cpp index 7d2bcfa712c5..e4db6a0f1e58 100644 --- a/mozglue/misc/decimal/Decimal.cpp +++ b/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; diff --git a/mozglue/misc/decimal/Decimal.h b/mozglue/misc/decimal/Decimal.h index 4bb9a841e585..8d9adbff4fb0 100644 --- a/mozglue/misc/decimal/Decimal.h +++ b/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: