stylo: css calc() ignores constant operands

RESOLVED FIXED in Firefox 57

Status

()

defect
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: spagoveanu, Assigned: emilio)

Tracking

({regression})

58 Branch
mozilla58
Points:
---
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox56 unaffected, firefox57 fixed, firefox58 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

2 years ago
Posted file 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.
happens only on stylo enabled.
Status: UNCONFIRMED → NEW
Component: General → CSS Parsing and Computation
Ever confirmed: true
Product: Firefox → Core
Summary: css calc() ignores constant operands → stylo: css calc() ignores constant operands
Keywords: regression
Priority: -- → P2
Assignee: nobody → josh
Does it also happen in 57? There's nothing that would've landed in the meantime that looks familiar.
This is a typo in the `ex` code:

  ret.ex = Some(ret.em.unwrap_or(0.) + ex * factor);
Assignee: josh → emilio
Should add a test...
This is also happening in 57, and that that fix should be uplifted when it lands to central.
https://hg.mozilla.org/mozilla-central/rev/c75889c4efb4
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Flags: in-testsuite?
Posted file 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
Attachment #8917600 - Flags: review+
Attachment #8917600 - Flags: approval-mozilla-beta?
Per the comment above, this is tested in servo/ and will get to mozilla-central eventually.
Flags: in-testsuite? → in-testsuite+
Comment on attachment 8917600 [details] [review]
Servo patch.

Stylo related, Beta57+
Attachment #8917600 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(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.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.