Skip to content

Commit

Permalink
fix: Implement additional logic in DOMString::set_best_representation…
Browse files Browse the repository at this point in the history 
…_of_the_floating_point_number in order to correct some failing tests related to -0 values. (#32272)
  • Loading branch information
@shanehandley
shanehandley committed May 13, 2024
1 parent 3d4fd0e commit 8eeb888
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 74 deletions.
13 changes: 12 additions & 1 deletion components/script/dom/bindings/str.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use chrono::{Datelike, TimeZone};
use cssparser::CowRcStr;
use html5ever::{LocalName, Namespace};
use lazy_static::lazy_static;
use num_traits::Zero;
use regex::Regex;
use servo_atoms::Atom;

Expand Down Expand Up @@ -456,10 +457,20 @@ impl DOMString {
None
}

/// Applies the same processing as `parse_floating_point_number` with some additional handling
/// according to ECMA's string conversion steps.
///
/// Used for specific elements when handling floating point values, namely the `number` and
/// `range` inputs, as well as `meter` and `progress` elements.
///
/// <https://html.spec.whatwg.org/multipage/#best-representation-of-the-number-as-a-floating-point-number>
/// <https://tc39.es/ecma262/#sec-numeric-types-number-tostring>
pub fn set_best_representation_of_the_floating_point_number(&mut self) {
if let Some(val) = self.parse_floating_point_number() {
self.0 = val.to_string();
// [tc39] Step 2: If x is either +0 or -0, return "0".
let parsed_value = if val.is_zero() { 0.0_f64 } else { val };

self.0 = parsed_value.to_string()
}
}

Expand Down
8 changes: 7 additions & 1 deletion components/script/dom/htmlinputelement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2189,7 +2189,13 @@ impl HTMLInputElement {
datetime.format("%Y-%m-%dT%H:%M:%S%.3f").to_string(),
))
},
InputType::Number | InputType::Range => Ok(DOMString::from(value.to_string())),
InputType::Number | InputType::Range => {
let mut value = DOMString::from(value.to_string());

value.set_best_representation_of_the_floating_point_number();

Ok(value)
},
// this won't be called from other input types
_ => unreachable!(),
}
Expand Down
37 changes: 31 additions & 6 deletions components/script/dom/htmlmeterelement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLMeterElementBinding::HTMLMeterE
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::num::Finite;
use crate::dom::bindings::root::{DomRoot, MutNullableDom};
use crate::dom::bindings::str::DOMString;
use crate::dom::document::Document;
use crate::dom::element::Element;
use crate::dom::htmlelement::HTMLElement;
Expand Down Expand Up @@ -75,8 +76,12 @@ impl HTMLMeterElementMethods for HTMLMeterElement {

/// <https://html.spec.whatwg.org/multipage/#dom-meter-value>
fn SetValue(&self, value: Finite<f64>) {
let mut string_value = DOMString::from_string((*value).to_string());

string_value.set_best_representation_of_the_floating_point_number();

self.upcast::<Element>()
.set_string_attribute(&local_name!("value"), (*value).to_string().into());
.set_string_attribute(&local_name!("value"), string_value);
}

/// <https://html.spec.whatwg.org/multipage/#concept-meter-minimum>
Expand All @@ -91,8 +96,12 @@ impl HTMLMeterElementMethods for HTMLMeterElement {

/// <https://html.spec.whatwg.org/multipage/#dom-meter-min>
fn SetMin(&self, value: Finite<f64>) {
let mut string_value = DOMString::from_string((*value).to_string());

string_value.set_best_representation_of_the_floating_point_number();

self.upcast::<Element>()
.set_string_attribute(&local_name!("min"), (*value).to_string().into());
.set_string_attribute(&local_name!("min"), string_value);
}

/// <https://html.spec.whatwg.org/multipage/#concept-meter-maximum>
Expand All @@ -108,8 +117,12 @@ impl HTMLMeterElementMethods for HTMLMeterElement {

/// <https://html.spec.whatwg.org/multipage/#concept-meter-maximum>
fn SetMax(&self, value: Finite<f64>) {
let mut string_value = DOMString::from_string((*value).to_string());

string_value.set_best_representation_of_the_floating_point_number();

self.upcast::<Element>()
.set_string_attribute(&local_name!("max"), (*value).to_string().into());
.set_string_attribute(&local_name!("max"), string_value);
}

/// <https://html.spec.whatwg.org/multipage/#concept-meter-low>
Expand All @@ -129,8 +142,12 @@ impl HTMLMeterElementMethods for HTMLMeterElement {

/// <https://html.spec.whatwg.org/multipage/#dom-meter-low>
fn SetLow(&self, value: Finite<f64>) {
let mut string_value = DOMString::from_string((*value).to_string());

string_value.set_best_representation_of_the_floating_point_number();

self.upcast::<Element>()
.set_string_attribute(&local_name!("low"), (*value).to_string().into());
.set_string_attribute(&local_name!("low"), string_value);
}

/// <https://html.spec.whatwg.org/multipage/#concept-meter-high>
Expand All @@ -154,8 +171,12 @@ impl HTMLMeterElementMethods for HTMLMeterElement {

/// <https://html.spec.whatwg.org/multipage/#dom-meter-high>
fn SetHigh(&self, value: Finite<f64>) {
let mut string_value = DOMString::from_string((*value).to_string());

string_value.set_best_representation_of_the_floating_point_number();

self.upcast::<Element>()
.set_string_attribute(&local_name!("high"), (*value).to_string().into());
.set_string_attribute(&local_name!("high"), string_value);
}

/// <https://html.spec.whatwg.org/multipage/#concept-meter-optimum>
Expand All @@ -175,7 +196,11 @@ impl HTMLMeterElementMethods for HTMLMeterElement {

/// <https://html.spec.whatwg.org/multipage/#dom-meter-optimum>
fn SetOptimum(&self, value: Finite<f64>) {
let mut string_value = DOMString::from_string((*value).to_string());

string_value.set_best_representation_of_the_floating_point_number();

self.upcast::<Element>()
.set_string_attribute(&local_name!("optimum"), (*value).to_string().into());
.set_string_attribute(&local_name!("optimum"), string_value);
}
}
23 changes: 17 additions & 6 deletions components/script/dom/htmlprogresselement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLProgressElementBinding::HTMLPro
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::num::Finite;
use crate::dom::bindings::root::{DomRoot, MutNullableDom};
use crate::dom::bindings::str::DOMString;
use crate::dom::document::Document;
use crate::dom::element::Element;
use crate::dom::htmlelement::HTMLElement;
Expand Down Expand Up @@ -75,11 +76,15 @@ impl HTMLProgressElementMethods for HTMLProgressElement {
})
}

// https://html.spec.whatwg.org/multipage/#dom-progress-value
/// <https://html.spec.whatwg.org/multipage/#dom-progress-value>
fn SetValue(&self, new_val: Finite<f64>) {
if 0.0 <= *new_val {
if *new_val >= 0.0 {
let mut string_value = DOMString::from_string((*new_val).to_string());

string_value.set_best_representation_of_the_floating_point_number();

self.upcast::<Element>()
.set_string_attribute(&local_name!("value"), (*new_val).to_string().into());
.set_string_attribute(&local_name!("value"), string_value);
}
}

Expand All @@ -100,10 +105,16 @@ impl HTMLProgressElementMethods for HTMLProgressElement {
})
}

// https://html.spec.whatwg.org/multipage/#dom-progress-max
/// <https://html.spec.whatwg.org/multipage/#dom-progress-max>
fn SetMax(&self, new_val: Finite<f64>) {
self.upcast::<Element>()
.set_string_attribute(&local_name!("max"), (*new_val).to_string().into());
if *new_val > 0.0 {
let mut string_value = DOMString::from_string((*new_val).to_string());

string_value.set_best_representation_of_the_floating_point_number();

self.upcast::<Element>()
.set_string_attribute(&local_name!("max"), string_value);
}
}

// https://html.spec.whatwg.org/multipage/#dom-progress-position
Expand Down
30 changes: 0 additions & 30 deletions tests/wpt/meta-legacy-layout/html/dom/reflection-forms.html.ini
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20181,39 +20181,21 @@
[meter.value: IDL get with DOM attribute unset]
expected: FAIL

[meter.value: IDL set to -0]
expected: FAIL

[meter.min: IDL get with DOM attribute unset]
expected: FAIL

[meter.min: IDL set to -0]
expected: FAIL

[meter.max: IDL get with DOM attribute unset]
expected: FAIL

[meter.max: IDL set to -0]
expected: FAIL

[meter.low: IDL get with DOM attribute unset]
expected: FAIL

[meter.low: IDL set to -0]
expected: FAIL

[meter.high: IDL get with DOM attribute unset]
expected: FAIL

[meter.high: IDL set to -0]
expected: FAIL

[meter.optimum: IDL get with DOM attribute unset]
expected: FAIL

[meter.optimum: IDL set to -0]
expected: FAIL

[optgroup.accessKey: setAttribute() to "5%"]
expected: FAIL

Expand Down Expand Up @@ -20802,18 +20784,6 @@
[progress.max: setAttribute() to "+100"]
expected: FAIL

[progress.max: IDL set to -10000000000]
expected: FAIL

[progress.max: IDL set to -1]
expected: FAIL

[progress.max: IDL set to -0]
expected: FAIL

[progress.max: IDL set to 0]
expected: FAIL

[progress.max: IDL set to 1e-10]
expected: FAIL

Expand Down
30 changes: 0 additions & 30 deletions tests/wpt/meta/html/dom/reflection-forms.html.ini
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3380,24 +3380,6 @@
[meter.tabIndex: IDL set to -2147483648]
expected: FAIL

[meter.value: IDL set to -0]
expected: FAIL

[meter.min: IDL set to -0]
expected: FAIL

[meter.max: IDL set to -0]
expected: FAIL

[meter.low: IDL set to -0]
expected: FAIL

[meter.high: IDL set to -0]
expected: FAIL

[meter.optimum: IDL set to -0]
expected: FAIL

[form.tabIndex: setAttribute() to "7\\v"]
expected: FAIL

Expand Down Expand Up @@ -3551,18 +3533,6 @@
[progress.max: setAttribute() to "+100"]
expected: FAIL

[progress.max: IDL set to -10000000000]
expected: FAIL

[progress.max: IDL set to -1]
expected: FAIL

[progress.max: IDL set to -0]
expected: FAIL

[progress.max: IDL set to 0]
expected: FAIL

[progress.max: IDL set to 1e-10]
expected: FAIL

Expand Down

0 comments on commit 8eeb888

Please sign in to comment.