124603 - DND bookmark: one feedback line & two drop possibilities

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Pierre Chanial]

Linux build 2002 02 08 21

Let's say you bookmark list is:
Folder
 > Goat1
 > Goat2
 > Goat3
 > Goat4

The two following actions won't lead to the same result:
1) drag Goat1 between Goat2 and Goat3 with the mouse pointer closer to Goat2
2) drag Goat1 between Goat2 and Goat3 with the mouse pointer closer to Goat3

During ways 1 and 2, as expected, the DND feedback line appears between Goat2
and Goat3.

Expected results: 1 and 2 should give the same result i.e. Goat 1 inserted
between Goat2 and Goat3.
Actual results  : only 1) performs the correct behavior; 2) will append Goat1
between Goat3 and Goat4

Comment 1

[[not reading bugmail]]

confirming in 2-9-08 w2k, I see this, I think that bug 121241 just visually puts
the insertion bars closer together on down, but it doesn't appear to actually
fix the problem with insertion correctly.

Comment 2

[[not reading bugmail]]

it the mouse pointer is hovered over Goat 2, it will insert between Goat 2 &
Goat 3, if mouse pointer is hovered over Goat 3, it will insert between Goat 3 &
Goat 4.  

Comment 3

[Pierre Chanial]

nominating, since this bug is really annoying for the users that manage their
bookmarks in the sidebar and has no easy workaround.
CC'ing Trudelle and Blake

Comment 4

[Peter Trudelle]

bookmark management is provided in the Manage Bookmarks window, it is explicitly
not a goal of the bookmarks sidebar panel.

Comment 5

[Pierre Chanial]

Peter:
1) this bug occur in both the bookmark sidebar and manager.
2) It occurs also when you drag the proxy icon (in the location bar) and drop it
in the bookmark sidebar or manager

Comment 6

[Peter Trudelle]

nsbeta1- per Nav triage team

Comment 7

[Pierre Chanial]

Attachment: patch v1

Basically, the index dropIx relative to the container was not updated and the
use of orientation was wrong.
This patch has been tested for 1 and several selected items
Requesting r=blake, sr=ben

Comment 8

[Pierre Chanial]

Attachment: patch v2

Just removing two lines that I forgot to delete

Comment 9

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

Comment on attachment 72342 [details] [diff] [review]
patch v2

r=[e-mail]

Comment 10

[Blake Ross]

Comment on attachment 72342 [details] [diff] [review]
patch v2

r=blake
Test well.

Comment 11

[Peter Trudelle]

Pierre, please do not remove keywords you didn't add.

Marking nsbeta1+/1.0.

Comment 12

[Pierre Chanial]

Attachment: patch v3

Sorry, Peter.
More robust patch. the latter was failing in rare cases (multiple selection
dropped at its borders).
this patch also fixes bug 124601 and bug 121245.

Comment 13

[Pierre Chanial]

Attachment: patch v4

sanity check

Comment 14

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

Comment on attachment 73139 [details] [diff] [review]
patch v4

sr=[e-mail]

Comment 15

[[not reading bugmail]]

Pierre, have you tested this thouroughly yet? I was going to ask for approval,
but  I wanna make sure you've have done that, and know its working.. take a look
here at the bottom of 3-13 weekly status.. regarding Asa's notes for an approval
request.. thanks.  

here is the link from main page: http://www.mozilla.org/status/2002-03-13.html

Comment 16

[Pierre Chanial]

Dennis, I tested it as thoroughly as I could, but I encourage you to test it
against the 0.9.9 milestone or a build before 20020312. You just have to
download patchmaker (http://www.gerv.net/software/patch-maker/build-mode.html)
to do it. If you find a problem, first check that the patch introduce a real
regression by checking if this problem was not already present.
I am waiting blake for reviewing this patch, but I understand he's really busy
with the download manager.
I then will ask drivers for approval. The trouble is that this patch is against
bookmarks.xml,rev 1.31 and has already been obsoleted by the check in of bug
128815. I did not merge the patch to the tip because  it would be soon obsoleted
by bug 126193 (has r=, sr= and a=) and bug 119599.

Ben: what is the right order that I have to follow:
- asking for review then approval.
- waiting for bug 126193 and bug 119599 to be checked in.
- merge the patch to the tip

Comment 17

[Jaime Rodriguez, Jr.]

nsbeta1- per ADT triage.

Comment 18

[Peter Trudelle]

->[e-mail], cc ben

Comment 19

[[not reading bugmail]]

Patchmaker appear to be for UI XUL bugs only.. I tried to load up patch v4
patch, but I get patchmaker saying: patch is not suitable for use with
patchmaker. :no suitable match for <blah>../bookmarks.xml.  Sorry.  

Comment 20

[[not reading bugmail]]

oh yeah pierre, in case you haven't looked/read yet:
http://www.mozilla.org/hacking/reviewers.html - this shall help, as its
necessary to review a patch that is also merged with the tip as well. 
especially if you your code will be outdated like you pointed out by other
checkins for the same files. 

Comment 21

[Pierre Chanial]

Attachment: patch v4.1

patch updated to the tip
I also removed an if statement that was preventing DND just above and just
below the selected item. This is now not necessary since there is no more a
reflow for both the deletion and the insertion.
requesting r= and sr= again

this patch should be tested in the BM not the sidebar, since DND is broken in
the latter for the moment.

Comment 22

[[not reading bugmail]]

latest patch to bug 119599 has gone.  I think you should be ok if you diff it
again to the new file version in build 4-1.. 

-dennis

Comment 23

[Pierre Chanial]

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

Comment 24

[Pierre Chanial]

Attachment: patch v4.2

necessary update to the patch due to the outliner->tree conversion.
I have been running this patch for more than one month (patch v3), and noticed
no regression.
Requesting r=, sr=

Comment 25

[Pierre Chanial]

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

Comment 26

[Pierre Chanial]

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

Comment 27

[Pierre Chanial]

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

Comment 28

[Pierre Chanial]

fixed on trunk by the check in of bug 137119

Comment 29

[Pierre Chanial]

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

Comment 30

[Pierre Chanial]

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