723842 - Cut&paste of multiple bookmarks does not insert them contiguously

Status: RESOLVED WORKSFORME

(Firefox :: Bookmarks & History, defect)

Description

[Rick]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:10.0) Gecko/20100101 Firefox/10.0
Build ID: 20120123235200

Steps to reproduce:

0. Start in the bookmarks sidebar or the bookmarks library.
1. Start with two or more bookmarks in the same folder which you intend to move. In this example, use three and name then "1", "2", "3".
2. Have them followed (in the same folder) by a few stationary bookmarks, say three of them, and name them "A", "B", "C".
3. Using the keyboard, cut (not copy) and paste the numbered bookmarks to a position below (not above) their current position in the same folder. For example, paste them before "B".


Actual results:

The bookmarks are now incorrectly ordered: A, 1, B, 2, C, 3
The first bookmark is inserted correctly, but subsequent ones are not placed contiguously. Instead, it skips a bookmark before inserting the next one, skips another, inserts another, etc.


Expected results:

The bookmarks should be ordered: A, 1, 2, 3, B, C
The three pasted bookmarks (1, 2, 3) should be contiguous at the insertion point (before "B").
I tested this on ff10.0, aurora 11.0a2 (2012-01-31), nightly 13.0a1 (2012-02-02) on clean/new profiles.

Comment 1

[Alice0775 White]

Regression window
Cannot reproduce:
http://hg.mozilla.org/mozilla-central/rev/00bb08972e46
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110630 Firefox/7.0a1 ID:20110701112508
Can reproduce:
http://hg.mozilla.org/mozilla-central/rev/d1a0de2af8a7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110701 Firefox/7.0a1 ID:20110701141836
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=00bb08972e46&tochange=d1a0de2af8a7
Suspected:
4f84c26e07c2	Marco Bonardo — Bug 416459 - Implement proper cut action for bookmarks. r=dietrich

Comment 2

[Mark Banner (:standard8)]

The STR in comment 0 work for me in the latest FF 59 nightly. We also have unit test covering cut/paste and related items now, so I'm pretty sure this is ok to mark as WFM.

Comment 3

[Rick]

Mark, the STR as-is seems to work in 59 (though it still fails in 57), but if you copy instead of cut, you now get incorrect behavior where you didn't before, so perhaps the fix broke something else. It should be: 123ABC => 123A123BC, but you get 123321ABC (wrong position, and now wrong order (reversed) too).

Should this be re-opened?

Comment 4

[Mark Banner (:standard8)]

(In reply to Rick from comment #3)
> Mark, the STR as-is seems to work in 59 (though it still fails in 57), but
> if you copy instead of cut, you now get incorrect behavior where you didn't
> before, so perhaps the fix broke something else. It should be: 123ABC =>
> 123A123BC, but you get 123321ABC (wrong position, and now wrong order
> (reversed) too).

You're quite right, thank you for pointing that out.

> Should this be re-opened?

I think the simplest way is for me to address this in bug 1404909 - I'm changing the flows of how cut/copy/paste and drag/drop work there anyway, including fixing some more of this indexing items. I'm also making sure we have test coverage as well, which will help with getting this right.