137119 - [FIX] Dragging a bookmark in tree just below an open container jumps one row above it

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[perroe2]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.9+)
Gecko/20020411
BuildID:    20020411

When I try to drag a bookmark in a subfolder to the top of the subfolder, the
bookmark ends up one line to high. The same happens sometimes when I drag down too.

Reproducible: Sometimes
Steps to Reproduce:
1. Open "Manage Bookmarks" from the Bookmarks-folder.
2. Open a folder with around 10 bookmarks.
3. Try to move the last bookmark in the folder to the first line of the folder.

Actual Results:  Bookmark ends up a line to high, eg. outside the folder.

Expected Results:  Bookmark should end up at the place it was dragged to.

This happens about 1/2 of the times that I try.

Comment 1

[Gilles Durys]

*** This bug has been marked as a duplicate of 114606 ***

Comment 2

[Pierre Chanial]

Reopening, bug 114606 is a tracking bug, there is no need to dupe specific
issues against it.

Comment 3

[Pierre Chanial]

Marking new, and adding it to the dependency list of bug 114606

Comment 4

[beanladen]

Notoriously seen on Linux 0.9.9, too.

Comment 5

[Andreas Kunz]

Being annoyed by Bookmarks always appearing where I didn't expect them after
drag-and-drop (in Personal toolbar and in "Manage Bookmarks" - mostly they
appear one line above from where I placed them - this is one of my most hated
bugs nowadays) I looked around in bugzilla - and found a real jungle (including
this bug here) of open bugs describing the same:

bug 114256 (bookmark gets into wrong folder after dnd) although related, this
particular aspect is fixed now, but at least one of the dups is more general and
not adressed yet (bug 114878).

bug 114606 (bookmark goes to position above) describes exactly what I see and
hate, but in the meanwhile this bug has been somewhat changed into a tracking
bug and now also adresses other bugs related to bookmark d'n'd. (however,
component is not changed to "Tracking")
Many bugs that also describe the exact behaviour are duped to this one (I'll
list them later) because of its desription, but now nobody cares anymore about
the described problem because this one is now considered a tracking bug by some
people...

bug 119879 (bookmark appears one line above after dnd) is 100% what I see and hate.

bug 127437 (wrong position after dnd in personal toolbar) is about the same
problem just at another place (still mostly off-by-one after dragging bookmark).
Has *lots* of duplicates.

bug 137119 (bookmark positioned one line too high after dragging) is about
bookmark manager again, still the same problem. Was duped against bug 114606
already (same summary) but later reopened (bug 114606 is supposed to be tracking).

bug 139471 is finally about general dnd cleanup and is also supposed to fix this
particular issue I'm talking about. Very recently worked on, but now
unfortunately removed from RC2 list.


So it looks like (almost!) all of these six open bugs are duplicates of one
another, but correct duping and resolving was prevented by halfheartedly
changing bug 114606 into a tracking bug.
In addition the underlying bug I speak about (bookmark d'n'd results in wrong
position) was reported in many other bugs which are now duped to one of the six
bugs above: bug 115420, bug 116076, bug 117134, bug 120245, bug 122599, bug
130702, bug 137119, bug 130453, bug 130836, bug 132390, bug 134656, bug 137117,
bug 140071, bug 140342 and bug 140971 are the ones I found.


As you can see from the dup count or simply from imagining a browser where
bookmarks do not materialize where they are dropped, this is a quite serious
problem (and in my humble opinion should not go into 1.0). 

I think it would be a rewarding task for someone with the appropriate
permissions to clean up this jungle a bit. Maybe by clearly (and correctly)
labeling a general tracking bug and a separate one for the one particular d'n'd
problem I'm talking about (and duping the others against it).
This cleanup would get us rid of several ASSIGNED and NEW bugs and prepare the
way for fixing this one annoying d'n'd issue. 
Bug 139471 unfortunately was taken off the RC2 list (bug 138000) by Asa because
of its complexity. A simple "fix this one bug" wouldn't have been, I think.

BTW: yes, this problem also occurs with 2002050706 (branch) and 2002050708 (trunk).

Comment 6

[Pierre Chanial]

The issue of this bug is that dropping below an open container does the same
thing that dropping below a close container. It ends up by dropping bookmark
outside its container.
Taking, since I have a fix.

Comment 7

[Pierre Chanial]

Attachment: Patch v1.0

This patch fixes two issues:
- the problem reported in bug 124603: the drop index was wrong depending on the
orientation. I increment it before the removal/insertion but using
removeElement with the "false" attribute, so that the index are not modified.
- the problem reported here is the conjunction of the previous problem and the
fact that over an open container with orientation == DROP_AFTER, the drop did
not occur inside the container.

r= and sr= needed.

Comment 8

[Boris 'pi' Piwinger]

AFAICS the patch does its job.

pi

Comment 9

[Pierre Chanial]

r=timeless (was pending Boris' testing)

Comment 10

[Boris 'pi' Piwinger]

Patch WFM in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0+) Gecko/20020527.

pi

Comment 11

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

Comment on attachment 84186 [details] [diff] [review]
Patch v1.0

sr=[e-mail]

Comment 12

[Pierre Chanial]

fixed on trunk

Comment 13

[Pierre Chanial]

nominating for 1.0.1, no regression have been reported.

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 84186 [details] [diff] [review]
Patch v1.0

please check into the 1.0.1 branch ASAP. once landed remove the 
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword

Comment 15

[Mike Shaver (:shaver emeritus)]

renewing approval for this bug/patch.  Please replace mozilla1.0.1+ with
fixed1.0.1 when you've landed on the branch, and verified1.0.1 when the fix has
been verified there.

Comment 16

[lchiang]

claudius - can you verify this bug fix in 1.01 branch?  When verified, pls
replace fixed1.0.1 keyword with verified1.0.1.  Thanks.