1001258 - "ASSERTION: MovePositionBy should always lead to the movement specified, unless the frame is position:sticky", with position:relative, inline-block, & dynamic change to 'float'

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: MovePositionBy should always lead to the movement specified, unless the frame is position:sticky: 'StyleDisplay()->mPosition == NS_STYLE_POSITION_STICKY || GetPosition() + aTranslation == position', file layout/generic/nsFrame.cpp, line 5164

This assertion was added in https://hg.mozilla.org/mozilla-central/rev/e700c4f7f8e3

Comment 1

[Jesse Ruderman]

It took me a long time to reduce this testcase, so I hope it's useful!

Comment 2

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

So there's a codepath that moved the position:relative frame without calling its ApplyRelativePositioning method, which means that if that frame had been position:sticky instead of position;relative, the layout would have been wrong.  The question is which codepath...

Comment 3

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

The problem is that we change the rectangle to a reasonably-bogus value here:

#0  0x00007f1a70e0dd73 in nsIFrame::SetRect (this=0x5aaa2f8, aRect=...)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsIFrame.h:774
#1  0x00007f1a70e0ddaf in nsIFrame::SetRect (this=0x5aaa2f8, aWritingMode=..., 
    aRect=..., aContainerWidth=<optimized out>)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsIFrame.h:790
#2  0x00007f1a70e0c70a in nsLineLayout::BlockDirAlignLine (this=0x7fff57649ad8)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsLineLayout.cpp:1462
#3  0x00007f1a70e2fafc in nsBlockFrame::PlaceLine (this=0x5aa8a90, aState=..., 
    aLineLayout=..., aLine=..., aFloatStateBeforeLine=0x7fff57649be8, 
    aFloatAvailableSpace=..., aAvailableSpaceHeight=@0x7fff57649bfc: 0, 
    aKeepReflowGoing=0x7fff57649f0e)
    at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/generic/nsBlockFrame.cpp:4181

and then PlaceLine returns here:

  // If the available space between the floats is smaller now that we
  // know the height, return false (and cause another pass with
  // LINE_REFLOW_REDO_MORE_FLOATS).
  if (AvailableSpaceShrunk(oldFloatAvailableSpace, aFloatAvailableSpace)) {
    return false;
  }

before calling aLineLayout.RelativePositionFrames().

Comment 4

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

It's possible that the answer here is that we just remove the assertion, though.

Comment 5

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

(The problem here is that fundamentally, nsBlockFrame is bailing out in the middle of work that it knows it's going to redo.)

Comment 6

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

Attachment: Remove assertion added in bug 911786 part 3 since it turns out the first bug it found was bogus

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 8413142 [details] [diff] [review]
Remove assertion added in bug 911786 part 3 since it turns out the first bug it found was bogus

Maybe include the testcase as a crashtest, too?

r=me

Comment 8

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/14b4ebf888f3

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/14b4ebf888f3