366952 - "ASSERTION: used padding property missing (out of memory?)" involving -moz-inline-grid and position: fixed

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Loading this testcase triggers:

###!!! ASSERTION: used padding property missing (out of memory?): 'p', file /Users/admin/trunk/mozilla/layout/generic/nsFrame.cpp, line 731

Comment 1

[Boris Zbarsky [:bzbarsky]]

So the problem is that in nsHTMLReflowState::InitConstraints for the div with the padding-right there is no parentReflowState, so we never call InitOffsets(), so never compute the padding...

This is the "Sort out the behavior of nsHTMLReflowState::InitConstraints when parentReflowState is null. This happens for reflow roots and for frames being reflowed as a child of a box (via nsFrame::BoxReflow). The current behavior seems wrong." item in the to-do list on http://wiki.mozilla.org/Gecko:Reflow_Refactoring.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: possible patch (untested)

This is untested; I originally wrote it while working on bug 359769.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This patch breaks a bunch of things (HTTP auth dialogs to narrow, scrollbars a pixel or two off), as I'd noticed the previous time I tried doing this.  Need to figure out why.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This starts constructing the padding and border on the reflow state properly for reflow states with no parent, and switches the box/block adaptation code to using the reflow state border/padding on the block inside rather than the box's notion of border/padding, which fixes the regressions I was seeing without that change (and makes sense).

Comment 5

[Boris Zbarsky [:bzbarsky]]

Shouldn't we also ensure that the computed width/height doesn't go below 0?  Or are we guaranteed that it can't?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Yep.

This addresses Boris's comment.

Comment 7

[Boris Zbarsky [:bzbarsky]]

What about the 4th hunk in nsFrame::BoxReflow and the nsHTMLReflowState changes?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Right.

Comment 9

[Boris Zbarsky [:bzbarsky]]

r=bzbarsky, for what it's worth.  But I'm not as familiar with the box code as you guys, so it's probably still worth roc looking...

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk.

Comment 11

[Jesse Ruderman]

Crashtest checked in.

Comment 12

[[e-mail]]

Comment on attachment 251703 [details] [diff] [review]
patch

>+    InitOffsets(aContainingBlockWidth, aBorder, aPadding);
>+    // Override mComputedMargin since reflow roots start from the
>+    // frame's boundary, which is inside the margin.
>     mComputedMargin.SizeTo(0, 0, 0, 0);
>-    mComputedPadding.SizeTo(0, 0, 0, 0);
>-    mComputedBorderPadding.SizeTo(0, 0, 0, 0);
This caused bug 391358.
(I verified by manually applying these specific changes to a working build.)