80833 - Every other bookmark does not get deleted when attempting to delete multiple bookmarks

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect, P2)

Description

[Jon Rubin]

Found in 2001050706 trunk build on WinMe-ja; reproduced on Win98-en using 
2001042506 trunk build.

When selecting a list of bookmarks to delete in "Manage Bookmarks", every other 
bookmark fails to get deleted.  

Steps:
1) Create several bookmarks (i.e. 6 total).
2) In "Manage Bookmarks", select all of the bookmarks you just created and 
Delete them (rt-click, select delete).
3) The bookmarks no longer appear in the "Manage Bookmarks" list; however, half 
of the bookmarks still appear when selecting "Bookmarks" from the menu (so if 6 
bookmarks were originally added, the 2nd, 4th, and last bookmark are still 
there).  Closing "Manage Bookmarks" and reopening results in the undeleted 
bookmarks appearing there too.

Comment 1

[Ben Goodger (use ben at mozilla dot org for email)]

This is rather embarrassing, and I know what's going on. Need for rtm. 

Comment 2

[Todd Pringle]

Yes, this sucks. Adding nsbeta1 keyword.

Comment 3

[Stefan Förster]

Here the same problem (PC, WinME, Build 2001052404).

Comment 4

[doctor__j]

*** Bug 82847 has been marked as a duplicate of this bug. ***

Comment 5

[James Green]

X-Platform bug.

Comment 6

[Ben Sandberg]

Confirmed on Win2K, build 20010505.

Comment 7

[Ben Goodger (use ben at mozilla dot org for email)]

boink

Comment 8

[Jon Rubin]

Confirmed on Linux RH6.2-Ja, build 2001052205.

Comment 9

[Ben Goodger (use ben at mozilla dot org for email)]

*** Bug 77161 has been marked as a duplicate of this bug. ***

Comment 10

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch to fix

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Patch: 

- no need to increment |count| as we remove each item from the array as we 
process it (I believe this bug was introduced at a point where aSelection 
changed from an immutable nodelist to a mutable jsarray). 
- also, shift the removal until after the code that forcably removes the node 
from the document. the try .. catch was likely shielding a js error that must 
have been occurring here. 

Comment 12

[Blake Ross]

sr=blake

Comment 13

[Peter "jag" Annema]

r=jag

Comment 14

[Ben Goodger (use ben at mozilla dot org for email)]

Fix checked in. 

Comment 15

[Claudius Gayle]

VERIFIED Fixed with 2001061514 builds