Closed Bug 195355 Opened 22 years ago Closed 20 years ago

listBoxObject.getRowCount() returns wrong value


(Core :: XUL, defect)

Windows 2000
Not set





(Reporter: shanec, Assigned: neil)



(Keywords: fixed1.7)


(4 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030210
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030210

I discovered this while looking into keyboard navigation handling, I was getting
some exceptions in listboxes:

Komodo: Debug: (0): uncaught exception: [Exception... "Component returned failur
e code: 0x80004005 (NS_ERROR_FAILURE) [nsIListBoxObject.getItemAtIndex]"  nsresu
lt: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://global/cont
ent/bindings/listbox.xml#listbox.getItemAtIndex() :: getItemAtIndex :: line 1"
data: no]

This is reproducable with a simple listbox, just select the first item, the
press the 'z' key to get the exception.

      <listitem label="Normal"/>
      <listitem label="Selected" selected="true" />
      <listitem label="Disabled" disabled="true" />
      <listitem label="Checkbox" class="listitem-iconic" type="checkbox" 
      <listitem label="Checked"  type="checkbox" checked="true" />

I tracked the exception down to the keypress handler in the listbox xbl,
rowCount is always higher than the actual rows in the listbox, then the call to
getItemAtIndex would of course fail.  This is with a build from moz latest
downloaded a few hours ago.

It's not causing me any major problems, at least for now :)

Reproducible: Always

Steps to Reproduce:
-> varga
Assignee: jaggernaut → varga
Ever confirmed: true
I've seen a similar problem, the row count returned by getRowCount() is one less
than the actual number of rows in the listbox. In my case I think the error is
caused by line 1328 in nsListBoxBodyFrame.cpp which doesn't ensure that the row
count is accurate before modifying it.
WFM, can you still reproduce it ?
It's probably dependent on the order things happen, but I always see it on the
mailnews attachment list, if the very first message with attachments has three
or more, then the list thinks it has one fewer, possibly because it adds the 3
to the default value of -1, which means "count will be computed later".
Hmm, no I'm not able to easily reproduce it now (trunk as of last week).
Attached patch possible patchSplinter Review
Neil, I guess you already have something similar in your tree, but could you
try to reproduce the problem with this patch ?
getRowCount() returns 5 for me.
getRowCount() can also return a value greater than that required.

See the boiled-down test case that shows this. Apologies,
I only tested this on 1.3 final so far. View with -chrome.
This file may assist in confirming your fix works.

I wonder if the listbox "tallest row" calculations are getting knotted
up by the layout imposed by the listbox's sibling tag. That would
account for both getRowCount() problems and occassional odd layout
problems, and would be a more general issue than just getRowCount().

hope this helps. - N.
Jan, actually the way I fixed this variant was to use
if (mRowCount >= 0)
(similarly for the -- version)
But your patch works too...
Blocks: 145019
FWIW, the too-large test case works for me under firebird 0.6.1 and win xp.

My own problem is that when using a template in a listbox (sorry don't have an
available test case) the numbers returns by this function vary if this is the
first time I open the window (in which I get the currect number) or its not (in
which I get one more then there is), since my base code is 1.2 I'm not sure that
it is relevent but from inspecting the code it seems that the code didn't change.
Blocks: 122180
see also bug 122180
I'm going to request blocking here since not only is there a patch here, it 
addresses the longstanding and much-duped bug 122180.

> Jan, actually the way I fixed this variant was to use
> if (mRowCount >= 0)
>   ++mRowCount;
> (similarly for the -- version)

That implies that mRowCount will never be incremented if it's negative -- is 
there a later call to ComputeRowCount(), as seen in Jan's patch, that takes care 
of that issue?
Flags: blocking1.7?
(In reply to comment #12)
>is there a later call to ComputeRowCount(), as seen in Jan's patch, that
>takes care of that issue?
631 PRInt32
632 nsListBoxBodyFrame::GetRowCount()
633 {
634   if (mRowCount < 0)
635     ComputeTotalRowCount();
636   return mRowCount;
637 }
I think Neil's approach is better. We might need to add this check for
mRowCount-- too.
Assignee: varga →
Attached patch Proposed patchSplinter Review
Attachment #146703 - Flags: review?(varga)
Attachment #146703 - Flags: review?(varga) → review+
Attachment #146703 - Flags: superreview+
Comment on attachment 146703 [details] [diff] [review]
Proposed patch

Low-risk fix for annoying attachment display issue.
Attachment #146703 - Flags: approval1.7?
Fix checked in to the trunk.
Closed: 20 years ago
Resolution: --- → FIXED
Comment on attachment 146703 [details] [diff] [review]
Proposed patch

a=asa (on behalf of drivers) for checkin to 1.7
Attachment #146703 - Flags: approval1.7? → approval1.7+
Fix checked into the 1.7 branch.
Keywords: fixed1.7
Flags: blocking1.7?
*** Bug 214685 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.