416088 - Divide by zero [@ nsColumnSetFrame::ChooseColumnStrategy]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

Comment 1

[Mats Palmgren (inactive)]

Attachment: stack

Comment 2

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

The first part is just to make the code easier to read by having the
terms in the same order for this recurring expression.

Comment 3

[Mats Palmgren (inactive)]

BTW, maybe we should enforce that column gaps are positive?
That's what the CSS3 multicol draft says anyway:
http://www.w3.org/TR/css3-multicol/#lsquo2

Comment 4

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

This patch rejects specified negative column-gap and column-width.
The draft spec does not explicitly say that column-width cannot be negative,
but I think the text implies it:
"column width means the length of the line boxes"
http://www.w3.org/TR/css3-multicol/#lsquo

Comment 5

[fantasai]

The latest internal spec does indeed specify that column-gap and
column-rule-width must be non-negative. Furthermore, column-width and
column-count values must be greater than zero, so zero values should also be
thrown out at parse time.

Comment 6

[Mike Schroepfer]

Can we make a crash or reftest for this?

Comment 7

[Damon Sicore (:damons)]

Waiting to hear on the test before approving.

Comment 8

[Jesse Ruderman]

schrep and damon, the usual procedure for layout crashers is to check in the testcase as a crashtest at the same time as the patch or shortly after.  The testcase doesn't use scripts, so it can go in as a crashtest as-is.

If you think this should get additional tests to make sure negative -moz-column-width is thrown out entirely, those should probably be part of the big CSS parsing mochitest.

Comment 9

[Mats Palmgren (inactive)]

I added "-1px" as an invalid value for -moz-column-gap/width in
property_database.js which makes the crashtest redundant.

Filed bug 418036 to followup on comment 5.

mozilla/layout/generic/nsColumnSetFrame.cpp 	3.48
mozilla/layout/style/nsCSSParser.cpp 	3.385
mozilla/layout/style/test/property_database.js 	1.26 

-> FIXED