Closed
Bug 1407092
Opened 7 years ago
Closed 7 years ago
stylo: css calc() ignores constant operands
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | --- | fixed |
firefox58 | --- | fixed |
People
(Reporter: spagoveanu, Assigned: emilio)
Details
(Keywords: regression)
Attachments
(2 files)
317 bytes,
text/html
|
Details | |
41 bytes,
text/x-github-pull-request
|
emilio
:
review+
ritu
:
approval-mozilla-beta+
|
Details | Review |
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•7 years ago
|
||
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
Updated•7 years ago
|
Keywords: regression
Priority: -- → P2
Updated•7 years ago
|
Assignee: nobody → josh
Assignee | ||
Comment 2•7 years ago
|
||
Does it also happen in 57? There's nothing that would've landed in the meantime that looks familiar.
Assignee | ||
Comment 3•7 years ago
|
||
This is a typo in the `ex` code: ret.ex = Some(ret.em.unwrap_or(0.) + ex * factor);
Assignee | ||
Comment 4•7 years ago
|
||
https://github.com/servo/servo/pull/18807
Updated•7 years ago
|
Assignee: josh → emilio
Comment 5•7 years ago
|
||
Should add a test...
Updated•7 years ago
|
status-firefox56:
--- → unaffected
status-firefox57:
--- → unaffected
status-firefox58:
--- → affected
Assignee | ||
Comment 6•7 years ago
|
||
This is also happening in 57, and that that fix should be uplifted when it lands to central.
Comment 7•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c75889c4efb4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•7 years ago
|
Flags: in-testsuite?
Assignee | ||
Comment 8•7 years ago
|
||
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?
Assignee | ||
Comment 9•7 years ago
|
||
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+
Comment 11•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/65861c4a6ba4
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 12•7 years ago
|
||
(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.
Description
•