1407092 - stylo: css calc() ignores constant operands

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[spagoveanu]

Attachment: nightly-bug.html

In some expressions involving constant operands (eg. "-2ex - 1ex"), calc seems to ignore anything but the last operand.

See attached file -- the red little box should stick outside the black one.

This is a new bug in 58 nightly, older versions are fine.

Comment 1

[Tooru Fujisawa [:arai]]

happens only on stylo enabled.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Does it also happen in 57? There's nothing that would've landed in the meantime that looks familiar.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

This is a typo in the `ex` code:

  ret.ex = Some(ret.em.unwrap_or(0.) + ex * factor);

Comment 4

[Emilio Cobos Álvarez [:emilio]]

https://github.com/servo/servo/pull/18807

Comment 5

[Boris Zbarsky [:bzbarsky]]

Should add a test...

Comment 6

[Emilio Cobos Álvarez [:emilio]]

This is also happening in 57, and that that fix should be uplifted when it lands to central.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

https://hg.mozilla.org/mozilla-central/rev/c75889c4efb4

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Servo patch.

Patch at https://hg.mozilla.org/mozilla-central/rev/c75889c4efb4 (reviewed upstream)

Approval Request Comment
[Feature/Bug causing the regression]: stylo
[User impact if declined]: incorrect calc resolution with ex units.
[Is this code covered by automated tests?]: Yes (added in servo/, will be imported into servo/ when WPT is synced in both projects).
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not
[Why is the change risky/not risky?]: It's literally a one-character typo fix.
[String changes made/needed]: none

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Per the comment above, this is tested in servo/ and will get to mozilla-central eventually.

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8917600 [details] [review]
Servo patch.

Stylo related, Beta57+

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/65861c4a6ba4

Comment 12

[Andrei Vaida [:avaida]]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #8)
> [Is this code covered by automated tests?]: Yes (added in servo/, will be
> imported into servo/ when WPT is synced in both projects).
> [Has the fix been verified in Nightly?]: no
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Emilio's assessment on manual testing needs and the fact that this fix has automated coverage.