Crash [@ nsRuleNode::ComputeColumnData] with -moz-column-rule-color: inherit

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jruderman, Assigned: bugs)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
x86_64
Mac OS X
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 693637 [details]
testcase (crashes Firefox when loaded)
(Reporter)

Comment 1

6 years ago
Created attachment 693638 [details]
stack
(Assignee)

Updated

6 years ago
Assignee: nobody → bugs
(Assignee)

Comment 3

6 years ago
Created attachment 693692 [details] [diff] [review]
Fix

Add a missing null-check
Attachment #693692 - Flags: review?(cam)
(Assignee)

Comment 4

6 years ago
Created attachment 693701 [details]
Bug fix.

Adds a missing null-check. (+ Better patch file with 8 lines of context.)
Attachment #693692 - Attachment is obsolete: true
Attachment #693692 - Flags: review?(cam)
Attachment #693701 - Flags: review?(cam)
(Assignee)

Comment 5

6 years ago
Created attachment 693702 [details] [diff] [review]
Bug fix.

Clicking the []patch checkbox this time.
Attachment #693701 - Attachment is obsolete: true
Attachment #693701 - Flags: review?(cam)
Attachment #693702 - Flags: review?(cam)
You can avoid uploading a new patch and update the "patch-ness" of the existing attachment in the attachment details page if you click the "(edit details)" link.
Comment on attachment 693702 [details] [diff] [review]
Bug fix.

In COMPUTE_START_RESET, if we find that presContext is null we construct a new (default) style struct for the parent's style to use those values.  I think if we take the else branch here if parent is null then we will compute the value to whatever column-rule-color is when mColumnRuleColorIsForeground is true.  I am not sure that mColumnRuleColor is valid in that case.  In nsStyleColumn's construct it's set to NS_RGB(0, 0, 0), but I don't know if it remains that colour.

In any case, this will be different from nsStyleColor's default mColor value, which is set to aPresContext->DefaultColor() in nsStyleColor's constructor.  I feel like we ought to be using that.  You can just create a local nsStyleColor to get this mColor out of.
Attachment #693702 - Flags: review?(cam)
(Assignee)

Comment 8

6 years ago
Created attachment 698091 [details] [diff] [review]
Updated patch per review comments.
Attachment #693702 - Attachment is obsolete: true
Attachment #698091 - Flags: review?(cam)
Comment on attachment 698091 [details] [diff] [review]
Updated patch per review comments.

Looks good.
Attachment #698091 - Flags: review?(cam) → review+
(Assignee)

Comment 10

6 years ago
https://hg.mozilla.org/mozilla-central/rev/3077141dd57f
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.