Allow more valid use of calc expressions

[dzbarsky]

[pcwalton]

@dzbarsky Is this ready for review?

[dzbarsky]

Yes, it's good to go.

[metajack]

r? @pcwalton

[SimonSapin]

Note that the first 19 commits of this are in #7185, which is good to go except that it needs to add support for the ch unit (which was added while the PR was waiting on my to review).

[SimonSapin]

See also #7496.

[bors-servo]

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

[dzbarsky]

Rebased this to apply cleanly.

[jdm]

Github disagrees.

[dzbarsky]

the var() stuff rotted this PR. Rebased again.

[frewsxcv]

@dzbarsky In case you missed it, there's a style check failure:

./components/style/values.rs:1353: missing space after ,

[SimonSapin]

Reviewed 1 of 20 files at r1, 8 of 19 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.

---

^{components/style/properties.mako.rs, line 1927 [r3] (raw file):}
Please remove the outdated comment. (It used to explain why percentages were replaced with lengths at parse time.)

---

^{components/style/values.rs, line 210 [r3] (raw file):}
It’s not great that this uses a Calc struct that has a percentage field while this Length type should not allow percentages, but I don’t have a good idea to suggest how to fix this without duplicating a lot of calc() code. What do you think?

---

^{components/style/values.rs, line 398 [r3] (raw file):}
This accepts fractional numbers in the input and implicitly rounds them to integers, doesn’t it? See <integer> v.s. <number> in https://drafts.csswg.org/css-values/#calc-type-checking . I think this can be solved by adding an Integer variant to CalcUnit.

---

^{components/style/values.rs, line 447 [r3] (raw file):}
Maybe rename this to CalcLengthOrPercentage?

---

^{components/style/values.rs, line 492 [r3] (raw file):}
Change this to Ok(Token::Delim('/')) if expected_unit != CalcUnit::Integer => {. (See previous comment.)

---

Comments from the review on Reviewable.io

[eefriedman]

@bors-servo retry #8258

[bors-servo]

⚡ Previous build results for android, gonk, linux-dev, mac-rel-wpt are reusable. Rebuilding only linux-rel, mac-dev-ref-unit, mac-rel-css...

[bors-servo]

💔 Test failed - mac-rel-css

[eefriedman]

I'm not sure exactly why, but /css21_dev/html4/font-146.htm is consistently failing.

[dzbarsky]

The problem is the following declaration: div.test { font: 4em/-2em serif; background: red; color: yellow; }
The text in the div should not be visible, because the line-height is negative. Previously, servo didn't know how to parse 4em for the font-size, so the declaration would be ignored. We are now parsing that part, but are not yet handling the line-height, so the text shows up and the test breaks. We should fix font parsing, but this is follow-up fodder.

[eefriedman]

Fine; I'll r+ if you update the expected result.

[dzbarsky]

Looks like I was wrong. The actual problem is that we lost the check for negative line heights...

[SimonSapin]

@bors-servo r=mbrubeck

[bors-servo]

📌 Commit 00980ea has been approved by mbrubeck

[bors-servo]

⌛ Testing commit 00980ea with merge e72bd14...

[bors-servo]

💔 Test failed - mac-rel-wpt

[SimonSapin]

Unexpected Results
==================

/2dcontext/drawing-images-to-the-canvas/drawimage_canvas_12.html
----------------------------------------------------------------
TIMEOUT [Parent]

Sounds maybe unrelated?

[SimonSapin]

@bors-servo retry

#8292

[bors-servo]

⚡ Previous build results for android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css are reusable. Rebuilding only mac-rel-wpt...

[bors-servo]

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt