Closed Bug 547338 Opened 11 years ago Closed 11 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: mats)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(3 files, 2 obsolete files)

Attached file testcase
###!!! 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
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) — 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) — 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 3Splinter 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
http://hg.mozilla.org/mozilla-central/rev/24b8e061eff0
http://hg.mozilla.org/mozilla-central/rev/01f5e4105dae
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs review]
Ouch, I committed the wrong patch for this bug...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
http://hg.mozilla.org/mozilla-central/rev/bc3152ee4dbf
http://hg.mozilla.org/mozilla-central/rev/52efa25f3458
Status: REOPENED → RESOLVED
Closed: 11 years ago11 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.