###!!! ASSERTION: No integers in range; 0 is supposed to be in range: 'low <= high', file /Users/jruderman/mozilla-central/layout/generic/nsGfxScrollFrame.cpp, line 1742 This assertion was added in rev 4ccff5df452c. user: Robert O'Callahan date: Thu Oct 08 16:01:15 2009 +1300 summary: Bug 526394. Part 31: Move scroll implementation into nsGfxScrollFrame. r=mats

The problem is the use of GetOverflowRect() during reflow - it's not updated yet to take into account the new frame size. The frame size was changed in stack frame #12, applying minimum height: http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#2527

Take the current frame size into account.

It seems that this could change the result of GetScrolledRect unnecessarily/unexpectedly. How about making nsListBoxBodyFrame::VerticalScroll/Layout set up the overflow area correctly before we call ScrollTo?

I believe nsListBoxLayout::Layout is the "culprit", so I added it there. I think this is the only caller of VerticalScroll during reflow so the overflow rect should be correct in all other cases.

+ friend class nsListBoxLayout; // needs SetOverflowRect Let's just make SetOverflowRect protected. + if (frame->HasOverflowRect()) { + nsRect overflowRect = nsRect(nsPoint(0, 0), frame->GetSize()); + overflowRect.UnionRect(overflowRect, frame->GetOverflowRect()); + frame->SetOverflowRect(overflowRect); Using the old, invalid overflow rect seems bad. Shouldn't this be in a post-reflow callback? The comments in bug 51084 suggest it used to be...

Comment on attachment 428360 [details] [diff] [review] Patch 2 See comment #5.

You're comment implies we don't need to vertical scroll before LayoutInternal? If so, I think we can just remove this. Looking at nsListBoxBodyFrame's reflow callback: http://mxr.mozilla.org/mozilla-central/source/layout/xul/base/src/nsListBoxBodyFrame.cpp#446 it already does VerticalScroll, and checks for a row height change afterward and if so schedules another reflow with a possible scroll request (mAdjustScroll) -- which seems to be what this block tries to fix, according to the comment. The patch pass TryServer unit tests, fwiw. I'm not sure there are any test that covers the original problem though (bug 51084). I did run Firefox and Thunderbird locally on OSX with the patch without seeing any problems.

s/You're/Your

http://hg.mozilla.org/mozilla-central/rev/24b8e061eff0 http://hg.mozilla.org/mozilla-central/rev/01f5e4105dae

Ouch, I committed the wrong patch for this bug...

http://hg.mozilla.org/mozilla-central/rev/bc3152ee4dbf http://hg.mozilla.org/mozilla-central/rev/52efa25f3458