Closed Bug 1379436 Opened 7 years ago Closed 7 years ago

calc(1*2*3) does not work as expected

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox56 --- fixed

People

(Reporter: emk, Assigned: emk)

References

Details

Attachments

(2 files)

line-height-calc.html
425 bytes, text/html
Details
Bug 1379436 - Add some reftests for line-height: calc().
59 bytes, text/x-review-board-request
xidorn
: review+
Details
Attached file line-height-calc.html — 
calc(1*2*3) does not work, but calc(1*2), calc(1*2*3*1px), calc((1*2)*3), calc(1*(2*3)), or calc((1*2*3)) works as expected. Chrome and Edge work as expected.

Oddly enough, my local build works as expected, but automation builds do not. (So mozregession did not help.)
Debug builds crashed with this test case. But How can I take a stack trace (or send a crash report) from a debug build?
Assertion failure: aValue.GetUnit() == eCSSUnit_Number (unexpected unit), at c:\builds\moz2_slave\m-cen-w64-d-000000000000000000\build\src\layout\style\CSSCalc.h:269
#01: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3ce3751]
#02: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d0cd34]
#03: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d39fda]
#04: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d1ae29]
#05: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3c20a9a]
#06: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d39f26]
#07: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x210b5cb]
#08: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x210b2e0]
#09: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3da82e7]
#10: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3da84f5]
#11: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3dd771b]
#12: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3da5b81]
#13: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3da8945]
#14: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3da8835]
#15: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3da755a]
#16: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3da83ca]
#17: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3da84f5]
#18: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3daabc2]
#19: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3db0727]
#20: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3db080f]
#21: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3db099a]
#22: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d7b889]
#23: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d5eded]
#24: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d4855d]
#25: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d496f2]
#26: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d49ae6]
#27: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d47e9b]
#28: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d46f9c]
#29: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d49994]
#30: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3d46ad5]
#31: ???[D:\Program Files\Nightly\xul.dll +0xeb1efd]
#32: ???[D:\Program Files\Nightly\xul.dll +0xeb0e3b]
#33: soundtouch::SoundTouch::operator=[D:\Program Files\Nightly\xul.dll +0x13ea6e7]
#34: ???[D:\Program Files\Nightly\xul.dll +0x13b0b36]
#35: ???[D:\Program Files\Nightly\xul.dll +0x13b07de]
#36: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x3853f97]
#37: mozilla_dump_image[D:\Program Files\Nightly\xul.dll +0x38c66bb]
#38: workerlz4_maxCompressedSize[D:\Program Files\Nightly\xul.dll +0x4fa5fa6]
#39: workerlz4_maxCompressedSize[D:\Program Files\Nightly\xul.dll +0x50546c1]
#40: workerlz4_maxCompressedSize[D:\Program Files\Nightly\xul.dll +0x5051175]
#41: workerlz4_maxCompressedSize[D:\Program Files\Nightly\xul.dll +0x505097c]
#42: ???[D:\Program Files\Nightly\firefox.exe +0x19db]
#43: ???[D:\Program Files\Nightly\firefox.exe +0x145f]
#44: ???[D:\Program Files\Nightly\firefox.exe +0x1da3]
#45: TargetNtUnmapViewOfSection[D:\Program Files\Nightly\firefox.exe +0x47229]
#46: BaseThreadInitThunk[C:\WINDOWS\System32\KERNEL32.DLL +0x12774]
#47: RtlUserThreadStart[C:\WINDOWS\SYSTEM32\ntdll.dll +0x70d51]
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Comment on attachment 8884625 [details]
Bug 1379436 - Add some reftests for line-height: calc().

https://reviewboard.mozilla.org/r/155496/#review160506

Strictly speaking, this fix is incorrect. It would reject `line-height: calc(-1)` because it simplifies it to `line-height: -1`, while this should be accepted and clamped to zero during computation per spec.

Why do you want to fix this at this moment? Does it work as expected in Stylo? If yes, I think we can just land the reftest and mark it `fails-if(!stylo||styloVsGecko)`, and file a different bug blocking 1365771. It doesn't seem to me this is particularly worth fixing for now.
Attachment #8884625 - Flags: review?(xidorn+moz)
Comment on attachment 8884625 [details]
Bug 1379436 - Add some reftests for line-height: calc().

https://reviewboard.mozilla.org/r/155496/#review160506

> Strictly speaking, this fix is incorrect. It would reject line-height: calc(-1) because it simplifies it to line-height: -1, while this should be accepted and clamped to zero during computation per spec.

It failed even without this patch in Gecko. But OK, I removed the Gecko fix and marked the test fails-if(!stylo||styloVsGecko). I also added a testcase for line-height: calc(-1).
Comment on attachment 8884625 [details]
Bug 1379436 - Add some reftests for line-height: calc().

https://reviewboard.mozilla.org/r/155496/#review160506

Well, it is parsed as expected (`line-height: calc(-1)` isn't rejected), but isn't clamped during computation as expected. That is probably a problem...
Comment on attachment 8884625 [details]
Bug 1379436 - Add some reftests for line-height: calc().

https://reviewboard.mozilla.org/r/155496/#review160514

r=me if try run goes fine.
Attachment #8884625 - Flags: review?(xidorn+moz) → review+
I had to skip the test for (debug builds && !stylo) because debug builds will crash.
See Also: → 1379467
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #4)
> Why do you want to fix this at this moment? Does it work as expected in
> Stylo? If yes, I think we can just land the reftest and mark it
> `fails-if(!stylo||styloVsGecko)`, and file a different bug blocking 1365771.

Filed bug 1379467.
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/fadd662244d3
Add some reftests for line-height: calc(). r=xidorn
https://hg.mozilla.org/mozilla-central/rev/fadd662244d3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: