Closed
Bug 68547
Opened 24 years ago
Closed 24 years ago
D&D of bookmarks to the top of their folder sometimes deletes them
Categories
(SeaMonkey :: Bookmarks & History, defect, P1)
SeaMonkey
Bookmarks & History
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9
People
(Reporter: fabian, Assigned: bugs)
References
Details
(Keywords: dataloss)
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win95; en-US; 0.8) Gecko/20010210
BuildID: 2001021020
This is BAD dataloss. In the bookmarks manager, dragging and dropping bookmarks
to the top of their own folder sometimes deletes them. I have not yet found a
way to reproduce this 100% but it's happened pretty often. Boris Zbarsky also
could reproduce it on linux.
Reproducible: Always
Steps to Reproduce:
1) Launch Mozilla
2) Bookmarks|Bookmarks Manager
3) Expand Mozilla Project folder
4) Expand Developer Information subfolder
5) Drag the items in that subfolder to the top of the "Developer Information"
folder (their own folder).
If you do this a number of times (sometimes it takes another folder to
reproduce), the folder or bookmarks disappears.
Actual Results: The bookmark/folder is deleted
Expected Results: successfully moved to the top of the folder
Happens on linux and win at least, suspect xp
Comment 2•24 years ago
|
||
The problem seems to occur when we go through the code marked
// XXX magical fix for bug # 33546: handle dropping after open container
in bookmarksDD.js.
Comment 3•24 years ago
|
||
JavaScript error:
uncaught exception ... NS_ERROR_ILLEGAL_VALUE ...
nsIRDFContainer.InsertElementAt ... location ... bookmarksDD.js :: anonymous ::
line 256 data:no
Lines 255/256 of bookmarksDD.js are
var dropIx = RDFC.IndexOf(rTarget);
RDFC.InsertElementAt(rSource, dropAction == "after" ? ++dropIx : dropIx, true);
When the error occurs dropIx is -1, i.e. rTarget is not in the RDFC container.
If I add a line to set dropIx to 0 if it is -1, then the bookmark moves to the
first item of the parent folder.
Hope that helps.
Comment 4•24 years ago
|
||
Comment 5•24 years ago
|
||
The above patch fixes the dataloss problem, I think. I'm now seeing a problem
where the bookmark disappears in the Bookmarks Manager window, but is shown as
moving correctly in the Sidebar. If I close and reopen Bookmarks Manager, it's
OK again. Tree display problem?
Comment 6•24 years ago
|
||
There's a band-aid patch here to fix some serious dataloss, but the underlying
problem needs looking into.
Gerv
Comment 7•24 years ago
|
||
The problem of the tree not refreshing seems not to occur on WinNT.
Assignee | ||
Comment 8•24 years ago
|
||
What is the effect of moving the rContainer line? I'm curious because the patch
you attached is not what you describe in your previous comment.
Status: NEW → ASSIGNED
Comment 9•24 years ago
|
||
I think this is what happens:
In the original code, we set ContainerItem, then set rContainer based on the
value of ContainerItem, and then possibly change ContainerItem (in the bug 33546
fix section of the code). In this case rContainer does not really point to the
container we are trying to put the item in, so the IndexOf fails, and then the
paste fails.
Comment 10•24 years ago
|
||
Marking nsbeta1+, p1, mozilla0.9
Comment 11•24 years ago
|
||
Richard: I'm CCing you on this because of your patch for bug 33546. Please
verify that Matthew's patch to your bugfix makes it do what you intended all along.
...
Matthew's patch (attachment 02 [details] [diff] [review]/22/01 14:51) is correct. It is not a band-aid
patch. Richard's fix for bug 33546 was meant to handle the case where the drop
target is the bottom quarter or so of a folder. In this case, the drop action
variables are calculated as:
dropAction = "after"
dropItem = target folder
containerItem = parent of container folder
Normally, this would insert the dropped item after the folder at the same level
as the folder. The "magical fix" changes this to:
dropAction = "before"
dropItem = first item of target folder
containerItem = target folder
if the target folder is open, so the dragged item gets stuck at the beginning of
the target folder, as expected.
Now, "dropItem" and "containerItem" become "rTarget" and "rContainer" like so:
var rContainer = this.getResource(NODE_ID(containerItem));
var rTarget = this.getResource(NODE_ID(dropItem));
HOWEVER, there's a bug (either in the original fix to introduced later).
"rContainer" is assigned before the magical fix changes the value of
"containerItem" whose value is not used again. Therefore, the algorithm
proceeds with the corrected values of "dropItem" and "rTarget" and the wrong
value of "rContainer". Chaos ensues! (That is, the dropped bookmark is lost
forever.)
This patch is obviously more correct than what's there. Is it too late to
keyword this with mozilla0.8.1?
Note that the redraw problem (where items disappear but return when you close
and reopen the bookmark manager) is a separate issue. It happens in other
contexts, too, and shouldn't stop us from getting this fix in.
Reporter | ||
Comment 12•24 years ago
|
||
r=fabian tested on windows95 on multiple mozilla versions, makes sense, and
comments by kevin were very useful :-)
Keywords: approval
Comment 13•24 years ago
|
||
This bug is still there in 2001041104 (win98). In sidebar D&D of a subfolder
from one folder to another resulted in losing the whole subfolder.
Assignee | ||
Comment 14•24 years ago
|
||
nerp
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 15•24 years ago
|
||
aside from the aforementioned separate redraw issues this works. VERIFIED Fixed with 2001042608
builds on all platforms.
Status: RESOLVED → VERIFIED
Comment 16•23 years ago
|
||
Reposting my comments from bug 56349, as it is more appropriate to this bug.
I'd hate for one to slip through the cracks...
This might be caused by the redraw issues, except I don't have to close Bookmark
Manager to fix it. Can I get a bug number for that bug? I either need to mark
this as depending on it, or re-open this one.
Win98 build 2001080603:
1)Open Bookmark Manager
2)Create a Folder
3)Create or Move 3 bookmarks into Folder
4)Drag and drop second bookmark onto gap between Folder and first bookmark
(after it highlights)
Actual Results: Bookmark disappears until Folder is closed, then reopened.
Expected Results: Bookmark moves to first position within Folder, no refresh needed.
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•