removeItemAt for <listbox> removes wrong item if there is a <listhead>

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
14 years ago
12 years ago

People

(Reporter: Bill Dimm, Assigned: jag (Peter Annema))

Tracking

({helpwanted})

Trunk
x86
Linux
helpwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(2 attachments)

693 bytes, application/vnd.mozilla.xul+xml
Details
fix
1.60 KB, patch
janv
: review+
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

14 years ago
User-Agent:       
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7a) Gecko/20040218

If a <listbox> has a <listhead>, calls to removeItemAt() remove the wrong row. 
The function seems to treat the <listhead> as item 0 and it treats the first
<listitem> as item 1, which is not consistent with how other functions like
getItemAtIndex() behave.  Here is some sample code:

-------
<script>
   function del()
      {
      var list = document.getElementById('mylist');
      list.removeItemAt(1);
      }
</script>

<listbox id='mylist'>
   <listhead>
      <listheader label='My List'/>
   </listhead>
   <listitem label='zero'/>
   <listitem label='one'/>
</listbox>
<button label='Del' onclick='del()'/>
--------

Clicking the button "Del" should remove the "one" item from the list, but it
removes the "zero" item instead.  If you remove the "<listhead>...</listehead>"
the problem goes away.

Is there a way to work around this that won't break when the bug is fixed (i.e.
simply adding 1 to the index isn't a good work around since it will break)?  I
tried looking at getIndexOfFirstVisibleRow(), but it returns 0 (instead of 1)
even when <listhead> is present.

Reproducible: Always
Steps to Reproduce:
1. Implement the code given in the details.
2. Push the "Del" button
3.

Actual Results:  
The "zero" item is removed.

Expected Results:  
The "one" item should be removed.
(Reporter)

Comment 1

13 years ago
Actually, the presence of <listhead> isn't the only thing that causes problems.
 <listcols> will do it to.

If you examine the JavaScript code for removeItemAt(index), you will find that
it attempts to remove this.childNodes[index], which is only the right thing to
do if ALL of the child nodes are "listitem" objects.  If you have a <listhead>
or <listcols>, the child node with index 0 is not a listitem, so the wrong child
is removed.

Comment 2

13 years ago
You are right, all the methods should getItemAtIndex instead of childNodes.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: helpwanted
Whiteboard: [good first bug]

Comment 3

13 years ago
Created attachment 155125 [details]
reporter's testcase

Comment 4

13 years ago
Created attachment 155126 [details] [diff] [review]
fix

according to comment 2

Updated

13 years ago
Attachment #155126 - Flags: superreview+

Updated

13 years ago
Attachment #155126 - Flags: review?

Updated

13 years ago
Attachment #155126 - Flags: review? → review?(varga)

Updated

13 years ago
Attachment #155126 - Flags: review?(varga) → review+

Comment 5

13 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
*** Bug 277498 has been marked as a duplicate of this bug. ***

Comment 7

12 years ago
This bug still exists in current versions of Firefox. (1.0.5)  Could someone
please reopen this bug for current version of Firefox?  If not, what should be
done?  A new bug filed?
This is not going to be backported to the 1.0.x stable branch, it will be in the
next releases of Firefox and SeaMonkey.
You need to log in before you can comment on or make changes to this bug.