Closed Bug 547338 Opened 15 years ago Closed 15 years ago

"ASSERTION: No integers in range; 0 is supposed to be in range" with xul listbox

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5
Tracking Status
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: jruderman, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: assertion, testcase)

Attachments

(3 files, 2 obsolete files)

Attached file testcase (deleted) —
###!!! 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
Attached file stack (deleted) —
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
Attached patch Patch 1 (obsolete) (deleted) — Splinter Review
Take the current frame size into account.
Assignee: nobody → matspal
Attachment #428246 - Flags: review?(roc)
Component: Graphics → Layout
OS: Mac OS X → All
QA Contact: thebes → layout
Hardware: x86 → All
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?
Attached patch Patch 2 (obsolete) (deleted) — Splinter Review
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.
Attachment #428246 - Attachment is obsolete: true
Attachment #428360 - Flags: review?(roc)
Attachment #428246 - Flags: review?(roc)
+ 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...
Whiteboard: [needs updated patch]
Attached patch Patch 3 (deleted) — Splinter Review
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.
Attachment #428360 - Attachment is obsolete: true
Attachment #439937 - Flags: review?(roc)
Whiteboard: [needs updated patch] → [needs review]
s/You're/Your
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs review]
Ouch, I committed the wrong patch for this bug...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: