Add parsing of double values in style attributes

[KiChjang]

This change is

[highfive]

Heads up! This PR modifies the following files:

• @bholley: components/style/attr.rs
• @KiChjang: components/script/dom/htmlfontelement.rs

[KiChjang]

$ bash etc/ci/check_no_unwrap.sh

components/compositing/constellation.rs:870:                          LoadData::new(Url::parse("about:failure").unwrap()));

components/compositing/constellation.rs:953:                .unwrap_or_else(|| Url::parse("about:blank").unwrap());

The command "bash etc/ci/check_no_unwrap.sh" exited with 1.

Not my fault!

[jdm]

Yep, #10610 clears that up.

[nox]

Nit: could it return Result<f64, ()> instead?

[nox]

Shouldn't that make new tests pass?

-S-awaiting-review +S-needs-code-changes

---

Reviewed 7 of 7 files at r1.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

components/servo/Cargo.lock, line 2193 [r1] (raw file):
You should probably use ./mach cargo-update, I'm pretty sure more lock files need to change.

---

components/style/attr.rs, line 82 [r1] (raw file):
Just use trim_matches to skip whitespace.

---

components/style/attr.rs, line 106 [r1] (raw file):
No need to recreate the iterator.

---

components/util/str.rs, line 166 [r1] (raw file):
Nit: can you make this a char?

---

components/util/str.rs, line 170 [r1] (raw file):
Why not just do c == '.' btw?

---

components/util/str.rs, line 196 [r1] (raw file):
Why do we need to return the number of digits which were read?

---

Comments from Reviewable

[KiChjang]

Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

components/style/attr.rs, line 79 [r1] (raw file):
Should I change parse_integer as well then?

---

components/style/attr.rs, line 106 [r1] (raw file):
Can't do that, the read_* functions take ownership of the iterator.

---

components/util/str.rs, line 196 [r1] (raw file):
Because these functions take ownership of the iterator, and so I need to keep track of where it stopped parsing.

---

Comments from Reviewable

[nox]

-S-awaiting-review +S-needs-code-changes

---

Reviewed 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

---

components/style/attr.rs, line 79 [r1] (raw file):
Would be nice yes.

---

components/style/attr.rs, line 106 [r1] (raw file):
Well, just pass &mut Peekable<I>, no?

---

components/util/str.rs, line 196 [r1] (raw file):
Just make them take &mut Peekable<I>.

---

components/util/str.rs, line 170 [r2] (raw file):
Nit: char.

---

Comments from Reviewable

[KiChjang]

Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

---

components/style/attr.rs, line 106 [r1] (raw file):
Ack, this actually causes my parsing to fail, because take_while() consumes the first element that doesn't satisfy the condition. I could use take_while_ref offered by the itertools crate though, should I use that?

---

Comments from Reviewable

[KiChjang]

Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

---

components/style/attr.rs, line 106 [r1] (raw file):
Doesn't seem to work either, the iterator needs to be cloneable in order to use .take_while_ref().

---

Comments from Reviewable

[bors-servo]

☔ The latest upstream changes (presumably #10745) made this pull request unmergeable. Please resolve the merge conflicts.

[nox]

Reviewed 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

components/style/attr.rs, line 106 [r1] (raw file):
I don't see a take_while call anywhere so I'm confused.

Anyway I don't think having to decode a string multiple times should be needed to just parse an integer string.

---

components/style/attr.rs, line 66 [r3] (raw file):
Shouldn't do_parse_integer return a Result itself? Feel free to file an E-Easy about that.

---

Comments from Reviewable

[KiChjang]

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

components/style/attr.rs, line 106 [r1] (raw file):
take_while is what's being used in read_integers and read_fractions. I don't think it's necessary either but due to the design of take_while, I can't track the last character that wasn't a digit.

---

Comments from Reviewable

[nox]

Seems good enough. Please file an issue about do_parse_integers' return type and take_while being used.

No WPT test?

-S-awaiting-review +S-awaiting-answer

---

Reviewed 10 of 10 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[nox]

Seems good enough. Please file an issue about do_parse_integers' return type and take_while being used.

No WPT test?

-S-awaiting-review +S-awaiting-answer

---

Reviewed 10 of 10 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[KiChjang]

I have another branch that actually utilizes the parse_double utility function (it's for parsing HTMLInputElement doubles), but for this PR, I've added a unit test instead since it's only a util crate modification.

[KiChjang]

Filed #10860 and #10861.

@bors-servo r=nox

[bors-servo]

📌 Commit 48b2e9c has been approved by nox

[bors-servo]

⌛ Testing commit 48b2e9c with merge ccba2d6...

[bors-servo]

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt