nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1842027 - Make <input type=number> localization faster. r=masayuki

Browse source 
Using the fast unlocalized parser first exposes a subtle difference
between the ICU parser (which uses `double`), and Decimal::fromString
(which has a larger range).

Check for iee754 finiteness explicitly, matching our previous behavior,
and the expectation of this subtest:

  https://searchfox.org/mozilla-central/rev/a3852ea8db25c759bc8b108aeec870d66c95452c/testing/web-platform/tests/html/semantics/forms/the-input-element/number.html#33

That check matches this Blink code:

  https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/blink/renderer/core/html/parser/html_parser_idioms.cc;l=103;drc=0d1bbe8de137aaae7c956a33249eb840a0191627

This patch avoids a bunch of number->string->number conversions in the
common case where the value comes from the value attribute and thus
parses fine without localization shenanigans.

Differential Revision: https://phabricator.services.mozilla.com/D183254
This commit is contained in:
Emilio Cobos Álvarez 2023-07-12 15:28:07 +00:00
parent 788c318976
commit bf2d9b9d37
10 changed files with 128 additions and 136 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

100
dom/html/HTMLInputElement.cpp
View file

@ -1586,18 +1586,25 @@ Decimal HTMLInputElement::StringToDecimal(const nsAString& aValue) {
}
NS_LossyConvertUTF16toASCII asciiString(aValue);
std::string stdString(asciiString.get(), asciiString.Length());
return Decimal::fromString(stdString);
auto decimal = Decimal::fromString(stdString);
if (!decimal.isFinite()) {
return Decimal::nan();
}
// Numbers are considered finite IEEE 754 Double-precision floating point
// values, but decimal supports a bigger range.
static const Decimal maxDouble =
Decimal::fromDouble(std::numeric_limits<double>::max());
if (decimal < -maxDouble || decimal > maxDouble) {
return Decimal::nan();
}
return decimal;
}
Decimal HTMLInputElement::GetValueAsDecimal() const {
Decimal decimalValue;
nsAutoString stringValue;
GetNonFileValueInternal(stringValue);
return !mInputType->ConvertStringToNumber(stringValue, decimalValue)
? Decimal::nan()
: decimalValue;
Decimal result = mInputType->ConvertStringToNumber(stringValue).mResult;
return result.isFinite() ? result : Decimal::nan();
}
void HTMLInputElement::SetValue(const nsAString& aValue, CallerType aCallerType,
@ -1872,8 +1879,8 @@ Decimal HTMLInputElement::GetMinimum() const {
nsAutoString minStr;
GetAttr(nsGkAtoms::min, minStr);
Decimal min;
return mInputType->ConvertStringToNumber(minStr, min) ? min : defaultMinimum;
Decimal min = mInputType->ConvertStringToNumber(minStr).mResult;
return min.isFinite() ? min : defaultMinimum;
}
Decimal HTMLInputElement::GetMaximum() const {
@ -1892,8 +1899,8 @@ Decimal HTMLInputElement::GetMaximum() const {
nsAutoString maxStr;
GetAttr(nsGkAtoms::max, maxStr);
Decimal max;
return mInputType->ConvertStringToNumber(maxStr, max) ? max : defaultMaximum;
Decimal max = mInputType->ConvertStringToNumber(maxStr).mResult;
return max.isFinite() ? max : defaultMaximum;
}
Decimal HTMLInputElement::GetStepBase() const {
@ -1901,22 +1908,23 @@ Decimal HTMLInputElement::GetStepBase() const {
mType == FormControlType::InputNumber ||
mType == FormControlType::InputRange,
"Check that kDefaultStepBase is correct for this new type");
Decimal stepBase;
// Do NOT use GetMinimum here - the spec says to use "the min content
// attribute", not "the minimum".
nsAutoString minStr;
if (GetAttr(nsGkAtoms::min, minStr) &&
mInputType->ConvertStringToNumber(minStr, stepBase)) {
return stepBase;
if (GetAttr(nsGkAtoms::min, minStr)) {
Decimal min = mInputType->ConvertStringToNumber(minStr).mResult;
if (min.isFinite()) {
return min;
}
}
// If @min is not a double, we should use @value.
nsAutoString valueStr;
if (GetAttr(nsGkAtoms::value, valueStr) &&
mInputType->ConvertStringToNumber(valueStr, stepBase)) {
return stepBase;
if (GetAttr(nsGkAtoms::value, valueStr)) {
Decimal value = mInputType->ConvertStringToNumber(valueStr).mResult;
if (value.isFinite()) {
return value;
}
}
if (mType == FormControlType::InputWeek) {
@ -4595,44 +4603,42 @@ void HTMLInputElement::SanitizeValue(nsAString& aValue,
return;
}
Decimal value;
bool ok = mInputType->ConvertStringToNumber(aValue, value);
if (!ok) {
InputType::StringToNumberResult result =
mInputType->ConvertStringToNumber(aValue);
if (!result.mResult.isFinite()) {
aValue.Truncate();
return;
}
nsAutoString sanitizedValue;
if (aForGetter == ForValueGetter::Yes) {
// If the default non-localized algorithm parses the value, then we're
// done, don't un-localize it, to avoid precision loss, and to preserve
// scientific notation as well for example.
//
// FIXME(emilio, bug 1622808): Localization should ideally be more
// input-preserving.
if (StringToDecimal(aValue).isFinite()) {
if (!result.mLocalized) {
return;
}
// For the <input type=number> value getter, we return the unlocalized
// value if it doesn't parse as StringToDecimal, for compat with other
// browsers.
char buf[32];
DebugOnly<bool> ok = value.toString(buf, ArrayLength(buf));
sanitizedValue.AssignASCII(buf);
DebugOnly<bool> ok = result.mResult.toString(buf, ArrayLength(buf));
aValue.AssignASCII(buf);
MOZ_ASSERT(ok, "buf not big enough");
} else {
mInputType->ConvertNumberToString(value, sanitizedValue);
// Otherwise we localize as needed, but if both the localized and
// unlocalized version parse, we just use the unlocalized one, to
// preserve the input as much as possible.
// Otherwise this SanitizeValue call is for display. We localize as
// needed, but if both the localized and unlocalized version parse with
// the generic parser, we just use the unlocalized one, to preserve the
// input as much as possible.
//
// FIXME(emilio, bug 1622808): Localization should ideally be more
// input-preserving.
if (StringToDecimal(sanitizedValue).isFinite()) {
nsAutoString localizedValue;
mInputType->ConvertNumberToString(result.mResult, localizedValue);
if (StringToDecimal(localizedValue).isFinite()) {
return;
}
aValue.Assign(localizedValue);
}
aValue.Assign(sanitizedValue);
} break;
case FormControlType::InputRange: {
Decimal minimum = GetMinimum();
@ -4645,9 +4651,8 @@ void HTMLInputElement::SanitizeValue(nsAString& aValue,
// parse out from aValue needs to be sanitized.
bool needSanitization = false;
Decimal value;
bool ok = mInputType->ConvertStringToNumber(aValue, value);
if (!ok) {
Decimal value = mInputType->ConvertStringToNumber(aValue).mResult;
if (!value.isFinite()) {
needSanitization = true;
// Set value to midway between minimum and maximum.
value = maximum <= minimum ? minimum
@ -6756,13 +6761,16 @@ int32_t HTMLInputElement::GetWrapCols() {
int32_t HTMLInputElement::GetRows() { return DEFAULT_ROWS; }
void HTMLInputElement::GetDefaultValueFromContent(nsAString& aValue) {
if (GetEditorState()) {
GetDefaultValue(aValue);
// This is called by the frame to show the value.
// We have to sanitize it when needed.
if (mDoneCreating) {
SanitizeValue(aValue);
}
if (!GetEditorState()) {
return;
}
GetDefaultValue(aValue);
// This is called by the frame to show the value.
// We have to sanitize it when needed.
// FIXME: This is also used from GetNonFileValueInternal where we might not
// need to sanitize some of the time.
if (mDoneCreating) {
SanitizeValue(aValue);
}
}

20
dom/html/TextControlState.cpp
View file

@ -2532,18 +2532,16 @@ void TextControlState::GetValue(nsAString& aValue, bool aIgnoreWrap) const {
} else {
mBoundFrame->ClearCachedValue();
}
} else if (!mTextCtrlElement->ValueChanged() || mValue.IsVoid()) {
// Use nsString to avoid copying string buffer at setting aValue.
nsString value;
mTextCtrlElement->GetDefaultValueFromContent(value);
// TODO: We should make default value not include \r.
nsContentUtils::PlatformToDOMLineBreaks(value);
aValue = std::move(value);
} else {
if (!mTextCtrlElement->ValueChanged() || mValue.IsVoid()) {
// Use nsString to avoid copying string buffer at setting aValue.
nsString value;
mTextCtrlElement->GetDefaultValueFromContent(value);
// TODO: We should make default value not include \r.
nsContentUtils::PlatformToDOMLineBreaks(value);
aValue = value;
} else {
aValue = mValue;
MOZ_ASSERT(aValue.FindChar(u'\r') == -1);
}
aValue = mValue;
MOZ_ASSERT(aValue.FindChar(u'\r') == -1);
}
}

65
dom/html/input/DateTimeInputTypes.cpp
View file

@ -160,20 +160,17 @@ nsresult DateInputType::GetBadInputMessage(nsAString& aMessage) {
mInputElement->OwnerDoc(), aMessage);
}
bool DateInputType::ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const {
auto DateInputType::ConvertStringToNumber(const nsAString& aValue) const
-> StringToNumberResult {
uint32_t year, month, day;
if (!ParseDate(aValue, &year, &month, &day)) {
return false;
return {};
}
JS::ClippedTime time = JS::TimeClip(JS::MakeDate(year, month - 1, day));
if (!time.isValid()) {
return false;
return {};
}
aResultValue = Decimal::fromDouble(time.toDouble());
return true;
return {Decimal::fromDouble(time.toDouble())};
}
bool DateInputType::ConvertNumberToString(Decimal aValue,
@ -205,15 +202,13 @@ nsresult TimeInputType::GetBadInputMessage(nsAString& aMessage) {
mInputElement->OwnerDoc(), aMessage);
}
bool TimeInputType::ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const {
auto TimeInputType::ConvertStringToNumber(const nsAString& aValue) const
-> StringToNumberResult {
uint32_t milliseconds;
if (!ParseTime(aValue, &milliseconds)) {
return false;
return {};
}
aResultValue = Decimal(int32_t(milliseconds));
return true;
return {Decimal(int32_t(milliseconds))};
}
bool TimeInputType::ConvertNumberToString(Decimal aValue,
@ -321,25 +316,21 @@ nsresult WeekInputType::GetBadInputMessage(nsAString& aMessage) {
mInputElement->OwnerDoc(), aMessage);
}
bool WeekInputType::ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const {
auto WeekInputType::ConvertStringToNumber(const nsAString& aValue) const
-> StringToNumberResult {
uint32_t year, week;
if (!ParseWeek(aValue, &year, &week)) {
return false;
return {};
}
if (year < kMinimumYear || year > kMaximumYear) {
return false;
return {};
}
// Maximum week is 275760-W37, the week of 275760-09-13.
if (year == kMaximumYear && week > kMaximumWeekInMaximumYear) {
return false;
return {};
}
double days = DaysSinceEpochFromWeek(year, week);
aResultValue = Decimal::fromDouble(days * kMsPerDay);
return true;
return {Decimal::fromDouble(days * kMsPerDay)};
}
bool WeekInputType::ConvertNumberToString(Decimal aValue,
@ -390,25 +381,24 @@ nsresult MonthInputType::GetBadInputMessage(nsAString& aMessage) {
mInputElement->OwnerDoc(), aMessage);
}
bool MonthInputType::ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const {
auto MonthInputType::ConvertStringToNumber(const nsAString& aValue) const
-> StringToNumberResult {
uint32_t year, month;
if (!ParseMonth(aValue, &year, &month)) {
return false;
return {};
}
if (year < kMinimumYear || year > kMaximumYear) {
return false;
return {};
}
// Maximum valid month is 275760-09.
if (year == kMaximumYear && month > kMaximumMonthInMaximumYear) {
return false;
return {};
}
int32_t months = MonthsSinceJan1970(year, month);
aResultValue = Decimal(int32_t(months));
return true;
return {Decimal(int32_t(months))};
}
bool MonthInputType::ConvertNumberToString(Decimal aValue,
@ -445,21 +435,18 @@ nsresult DateTimeLocalInputType::GetBadInputMessage(nsAString& aMessage) {
mInputElement->OwnerDoc(), aMessage);
}
bool DateTimeLocalInputType::ConvertStringToNumber(
nsAString& aValue, Decimal& aResultValue) const {
auto DateTimeLocalInputType::ConvertStringToNumber(
const nsAString& aValue) const -> StringToNumberResult {
uint32_t year, month, day, timeInMs;
if (!ParseDateTimeLocal(aValue, &year, &month, &day, &timeInMs)) {
return false;
return {};
}
JS::ClippedTime time =
JS::TimeClip(JS::MakeDate(year, month - 1, day, timeInMs));
if (!time.isValid()) {
return false;
return {};
}
aResultValue = Decimal::fromDouble(time.toDouble());
return true;
return {Decimal::fromDouble(time.toDouble())};
}
bool DateTimeLocalInputType::ConvertNumberToString(

15
dom/html/input/DateTimeInputTypes.h
View file

@ -62,8 +62,7 @@ class DateInputType : public DateTimeInputTypeBase {
nsresult GetBadInputMessage(nsAString& aMessage) override;
bool ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const override;
StringToNumberResult ConvertStringToNumber(const nsAString&) const override;
bool ConvertNumberToString(Decimal aValue,
nsAString& aResultString) const override;
@ -81,8 +80,7 @@ class TimeInputType : public DateTimeInputTypeBase {
nsresult GetBadInputMessage(nsAString& aMessage) override;
bool ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const override;
StringToNumberResult ConvertStringToNumber(const nsAString&) const override;
bool ConvertNumberToString(Decimal aValue,
nsAString& aResultString) const override;
bool IsRangeOverflow() const override;
@ -108,8 +106,7 @@ class WeekInputType : public DateTimeInputTypeBase {
}
nsresult GetBadInputMessage(nsAString& aMessage) override;
bool ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const override;
StringToNumberResult ConvertStringToNumber(const nsAString&) const override;
bool ConvertNumberToString(Decimal aValue,
nsAString& aResultString) const override;
@ -126,8 +123,7 @@ class MonthInputType : public DateTimeInputTypeBase {
}
nsresult GetBadInputMessage(nsAString& aMessage) override;
bool ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const override;
StringToNumberResult ConvertStringToNumber(const nsAString&) const override;
bool ConvertNumberToString(Decimal aValue,
nsAString& aResultString) const override;
@ -144,8 +140,7 @@ class DateTimeLocalInputType : public DateTimeInputTypeBase {
}
nsresult GetBadInputMessage(nsAString& aMessage) override;
bool ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const override;
StringToNumberResult ConvertStringToNumber(const nsAString&) const override;
bool ConvertNumberToString(Decimal aValue,
nsAString& aResultString) const override;

7
dom/html/input/InputType.cpp
View file

@ -286,11 +286,10 @@ nsresult InputType::GetBadInputMessage(nsAString& aMessage) {
return NS_ERROR_UNEXPECTED;
}
bool InputType::ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const {
auto InputType::ConvertStringToNumber(const nsAString& aValue) const
-> StringToNumberResult {
NS_WARNING("InputType::ConvertStringToNumber called");
return false;
return {};
}
bool InputType::ConvertNumberToString(Decimal aValue,

13
dom/html/input/InputType.h
View file

@ -84,11 +84,16 @@ class InputType {
* ie parse a date string to a timestamp if type=date,
* or parse a number string to its value if type=number.
* @param aValue the string to be parsed.
* @param aResultValue the number as a Decimal.
* @result whether the parsing was successful.
*/
virtual bool ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const;
struct StringToNumberResult {
// The result decimal. Successfully parsed if it's finite.
Decimal mResult = Decimal::nan();
// Whether the result required reading locale-dependent data (for input
// type=number), or the value parses using the regular HTML rules.
bool mLocalized = false;
};
virtual StringToNumberResult ConvertStringToNumber(
const nsAString& aValue) const;
/**
* Convert a Decimal to a string in a type specific way, ie convert a

26
dom/html/input/NumericInputTypes.cpp
View file

@ -77,10 +77,9 @@ nsresult NumericInputTypeBase::GetRangeUnderflowMessage(nsAString& aMessage) {
"FormValidationNumberRangeUnderflow", mInputElement->OwnerDoc(), minStr);
}
bool NumericInputTypeBase::ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const {
aResultValue = HTMLInputElement::StringToDecimal(aValue);
return aResultValue.isFinite();
auto NumericInputTypeBase::ConvertStringToNumber(const nsAString& aValue) const
-> StringToNumberResult {
return {HTMLInputElement::StringToDecimal(aValue)};
}
bool NumericInputTypeBase::ConvertNumberToString(
@ -117,15 +116,18 @@ bool NumberInputType::HasBadInput() const {
return !value.IsEmpty() && mInputElement->GetValueAsDecimal().isNaN();
}
bool NumberInputType::ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const {
ICUUtils::LanguageTagIterForContent langTagIter(mInputElement);
aResultValue =
Decimal::fromDouble(ICUUtils::ParseNumber(aValue, langTagIter));
if (aResultValue.isFinite()) {
return true;
auto NumberInputType::ConvertStringToNumber(const nsAString& aValue) const
-> StringToNumberResult {
auto result = NumericInputTypeBase::ConvertStringToNumber(aValue);
if (result.mResult.isFinite()) {
return result;
}
return NumericInputTypeBase::ConvertStringToNumber(aValue, aResultValue);
// Try to read the localized value from the user.
ICUUtils::LanguageTagIterForContent langTagIter(mInputElement);
result.mLocalized = true;
result.mResult =
Decimal::fromDouble(ICUUtils::ParseNumber(aValue, langTagIter));
return result;
}
bool NumberInputType::ConvertNumberToString(Decimal aValue,

7
dom/html/input/NumericInputTypes.h
View file

@ -22,8 +22,8 @@ class NumericInputTypeBase : public InputType {
nsresult GetRangeOverflowMessage(nsAString& aMessage) override;
nsresult GetRangeUnderflowMessage(nsAString& aMessage) override;
bool ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const override;
StringToNumberResult ConvertStringToNumber(
const nsAString& aValue) const override;
bool ConvertNumberToString(Decimal aValue,
nsAString& aResultString) const override;
@ -45,8 +45,7 @@ class NumberInputType final : public NumericInputTypeBase {
nsresult GetValueMissingMessage(nsAString& aMessage) override;
nsresult GetBadInputMessage(nsAString& aMessage) override;
bool ConvertStringToNumber(nsAString& aValue,
Decimal& aResultValue) const override;
StringToNumberResult ConvertStringToNumber(const nsAString&) const override;
bool ConvertNumberToString(Decimal aValue,
nsAString& aResultString) const override;

9
intl/unicharutil/util/ICUUtils.cpp
View file

@ -99,7 +99,7 @@ bool ICUUtils::LocalizeNumber(double aValue,
}
/* static */
double ICUUtils::ParseNumber(nsAString& aValue,
double ICUUtils::ParseNumber(const nsAString& aValue,
LanguageTagIterForContent& aLangTags) {
MOZ_ASSERT(aLangTags.IsAtStart(), "Don't call Next() before passing");
@ -107,7 +107,7 @@ double ICUUtils::ParseNumber(nsAString& aValue,
return std::numeric_limits<float>::quiet_NaN();
}
uint32_t length = aValue.Length();
const Span<const char16_t> value(aValue.BeginReading(), aValue.Length());
nsAutoCString langTag;
aLangTags.GetNext(langTag);
@ -122,11 +122,10 @@ double ICUUtils::ParseNumber(nsAString& aValue,
static_assert(sizeof(UChar) == 2 && sizeof(nsAString::char_type) == 2,
"Unexpected character size - the following cast is unsafe");
auto parseResult = np->ParseDouble(
mozilla::Span<const char16_t>(PromiseFlatString(aValue).get(), length));
auto parseResult = np->ParseDouble(value);
if (parseResult.isOk()) {
std::pair<double, int32_t> parsed = parseResult.unwrap();
if (parsed.second == static_cast<int32_t>(length)) {
if (parsed.second == static_cast<int32_t>(value.Length())) {
return parsed.first;
}
}

2
intl/unicharutil/util/ICUUtils.h
View file

@ -63,7 +63,7 @@ class ICUUtils {
* Parses the localized number that is serialized in aValue using aLangTags
* and returns the result as a double. Returns NaN on failure.
*/
static double ParseNumber(nsAString& aValue,
static double ParseNumber(const nsAString& aValue,
LanguageTagIterForContent& aLangTags);
static void AssignUCharArrayToString(UChar* aICUString, int32_t aLength,