264571 - new items created above currently selected item, instead of below

Status: RESOLVED INVALID

(Firefox :: Bookmarks & History, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This is a bug that's been filed before, but there was an attempted fix.  The
patch that'll be attached here is the bandaid that will ship with 1.0, that will
bring things back to the previously broken state -- bookmarks will be created
above the currently selected item, instead of below.

The reason for this is that we use the RDF Container InsertElementAt interface
here -- which, when given an index to insert at that's already occcupied, shifts
the old element forward, instead of putting this item after it.  This is a
bummer.  I tried to do some magic where the index was pre-incremented, but
InsertElementAt bails if you pass it an index that's greater than the number of
items in the container.   In either case, we have to know the item's exact
index, so that we can undo.  It's a huge pain in the ass.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 264571-bookmarks-bandaid-0.patch

The aforementioned bandaid.

Comment 2

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

Comment on attachment 162236 [details] [diff] [review]
264571-bookmarks-bandaid-0.patch

r+a=[e-mail]

Comment 3

[sairuh (rarely reading bugmail)]

when I paste an item, it also appears above the current selection. would that
also be fixed here, or is that expected behavior?

Comment 4

[Tracy Walker [:tracy]]

requesting blocking aviary 1.0 to get on radar to get the fix checked in.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

My bad; this already went in, forgot to mark it.

Comment 6

[sairuh (rarely reading bugmail)]

(In reply to comment #5)
> My bad; this already went in, forgot to mark it.

Vlad, when did the fix go in? I still see this issue in 2004102009-0.9+ when I
try to add a new item in the Bookmarks Manager and Sidebar (via context menu or
toolbar icon).

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Sorry, i'm not thinking right; this bug still exists, and it was put back due to
the bandaid patch.. the patch doesn't fix the bug; the bug itself won't be fixed
until after 1.0..

Comment 8

[Asa Dotzler [:asa]]

This bug seems to have an aviary branch checkin associated with it. If this has
landed on the aviary branch (as much as it's going to for 1.0) can you please
add the "fixed-aviary1.0" keyword? Thanks.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

The patch isn't a fix, it actually introduces the problem described in this bug
(but fixes worse bugs in the process).

Comment 10

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change

Comment 11

[Marco Bonardo [:mak]]

design and code for 3.x is quite different, so this is invalid for 3.x, creating above is the default in all our views actually.