167335 - Cut, copy and paste of bookmark failed

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[doctor__j]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2a) Gecko/20020907
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2a) Gecko/20020907

asdf

Reproducible: Always

Steps to Reproduce:
1. Open Bookmark manager
2. Select a bookmark.
3. Cut/copy it using either the menu command or keyboard shortcut.
4. Paste it on a different location/folder.


Actual Results:  
The cut bookmark became vanished.
The copied bookmark isn't in the clipboard at all.

Comment 1

[Chris Lyon]

Works for me, same build WinXP.

Comment 2

[Frank J Rossi Jr]

Useing 98 SE here 400 Mhz P2 Celeron Mozilla 1.1
I saw the same thing while moving bookmarks around. After pasteing into new
directory it did not show up there. 
By getting out of the bookmarks, then bringing them back up, the paste then
appeared. 
It seems that the bookmarks do not refresh their view after a paste.
This don't happen all the time.

Comment 3

[Barry Baxley]

I see the same thing when dragging and dropping bookmarks in the Bookmark
Manager.  About 25% of the time, the display fails to reflect that the bookmark
was moved.  It continues to show in the original location, but it can't be
dragged to a new location.  By exiting the Bookmark Manager and reopening it, I
can see that it has moved.  An Update or Refresh feature in the Bookmark Manager
(under View or Tools menu) might alleviate the problem.

Running Mozilla 1.1 on Windows 2000 SP3 (1.7GHz Intel P4).

Comment 4

[ratman]

using moz1.2a on winme.

i have no problems with copy/paste, but i cannot paste a bookmark after a cut. 
the url does copy to clipboard; the paste just doesn't work, even after
closing/reopening the manage bookmarks window.

Comment 5

[Sven Grull]

1.2a on Win 2000
I have sometimes the same problem as described in comment #4, but it is not
reproducible. After restarting mozilla the bug often disappears.

Comment 6

[ratman]

this behaviour i described appears to be 100% repeatable for me on moz1.2a, even
with a new profile, and even after restarting mozilla.  however, it is wfm on
moz1.1.  anyone know what may have changed?

Comment 7

[Chris Lyon]

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

Comment 8

[Chris Lyon]

confirming based on duplicates and adding duplicate keywords

Comment 9

[samlew_lksk]

Followed the link from bug#169382.

I've experienced another bookmark related problem/symptom and have a bit of
information about how older versions of Mozzila worked. 

When I click on the "bookmarks" entry from the top toolbar, the entry between
"Go" and "Tools" and put the mouse pointer on the bottom arrow of the displayed
bookmarks, the bookmarks only scroll up a few lines, then scrolling halts. I
noticed a similar problem after trying to drag a bookmark manually from the
bottom of the bookmark page. Dragging a bookmark and placing the mouse pointer
on the top line of the bookmark page,I found that scrolling up no longer occurs.

I went back to Mozilla 1.1 and found that the bookmark cut/paste/copy and the
above two bookmark scrolling issues work, both in ver 1.1 and in build 2002072104.

However, I found that all of the above issues fail in ver 1.2a and in the latest
build, 2002091908.

Comment 10

[Yen-Wei Liu]

I am using Mozilla 1.2a on a Traditional Chinese Win98. I am reorganzing my
fairly big bookmarks. To cut bookmarks is okay, the cut entries are in  the
clipboard, but I cannot paste them back. Undo is always grayed. So I lose my
entries...:) This bug is always there.

Comment 11

[Christopher D. Coleman]

Using 20020910 on Win 2k SP2
Watching the clipboard, cut and copy work fine for me either through menu or
keystrokes (Appears in the clipboard), but paste does not through either. Undo
is inactive after a cut. (GRRRR!) Dragging (move) works but ctrl dragging (copy)
does not (bookmark is left where it was). As mentioned in #9, window will not
scroll in either direction during any type of drag operation. Pasted material
does NOT appear after closing and reopening bookmark manager or maximizing and
normalizing the window. Happens always.

Have also noticed that bookmarks will sometimes be created when asked and
sometimes not. Does not appear to be depenent on the site. Cannot establish a
pattern on this one.

Are there any developers looking at this?

Comment 12

[Chris Lyon]

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

Comment 13

[Alexander Rabtchevich]

Just it was described in comment No. 4 - cutted bookmarks are not inserted
either with Shift-Ins or with paste from popup menu. NT4, build 2002100411 

Comment 14

[Thomas Brown]

Two more funny bookmark paste problems:

1.  You can't drag bookmark from URL bar to open "Manage bookmarks" window--it
looks like it will be pasted in the proper space but it never shows up.

2.  If you have attempted to drag a bookmark like this and then leave an
existing bookmark entry highlighted, hitting Control-D to bookmark the current
page will sometimes overwrite the highlighted bookmark!

Comment 15

[Chris Lyon]

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

Comment 16

[Chris Lyon]

OS->All
Platform->All
based on duplicate report

Comment 17

[Max Alekseyev]

Confirming the bug on OS/2 build 2002101008
Severity -> critical due to data loss

Comment 18

[Michael Gabriel]

woopie, just lost 20 bookmarks, confirming 2002101708 trunk win2k

Comment 19

[Serge Gautherie (:sgautherie)]

"Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.2b) Gecko/20021016"


I see some other bugs reported here: some of them are opened under other
numbers... (and that's how it is expected to be done !)


About this bug, my experience is:
*Folder CnP: works fine.
*Bookmark CnP: Cut seems to put the URL (at least) in the Windows clipboard, but
Paste is not working.

Very annoying !

Comment 20

[Jim Mackie]

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

Comment 21

[Kevin McGrath]

Mozilla 1.2b & Windows NT 4.0. I have this exact same CnP problem. You can drag
& drop the bookmarks OK but CnP fails no matter what you try. I was able to at
least save the bookmarks by pasting into a MS Word document. I even tried
creating a new bookmark folder and pasting from the MS Word app but that still
failed.

Comment 22

[Geraldo Amaral]

I was able to cut a bookmark but not to paste it back. I open composer, pasted
into it and the cut link was correctly pasted, but I can't paste it into
Bookmark Manager. I was running Mozilla 1.2b on Mandrake 8.2 Linux, kernel
version 2.4.18-6mdk

Comment 23

[Pierre Chanial]

Bug 129428 strikes again!
the patch in bug 145858 reverted the code we had before ben's fix.

Here is the technical point of this bug.
After cutting a bookmark that is not in several containers (so that
IsBookmarkedInternal would return false on it), it will not be possible to get
its type (see GetSynthesizedType).

I disagree with that. A resource should be accessed or modified no matter if it
is hooked or not in the bookmark graph.
btw, for phoenix, I restored the old behavior to deal with 'hidden' resources in
one case, but I missed this one.

rjc, why do you need to check if the resource is hooked for aggregation? Could
you be more specific? I would be glad to understand your motivations so that
what we check in will not conflict.

-> rjc, since I am really busy.

Comment 24

[jcoppens]

Lost quite a lot of bookmarks this way. Was doing c/p into folders,
and found out after the damage was done.

Mind: Not even the Undo is activated (enabled) after a Ctrl-X or Cut
command. No way to recover the bookmarks...

This bug is also present in mozilla1.2

Comment 25

[Max Alekseyev]

Undo in bookmarks is a subject of bug 53422

Comment 26

[Jeremy M. Dolan]

I could have sworn I filed a bug on this ages ago. Forgot Mozilla doesn't bother
to fix dataloss problems and just lost another 20 bookmarks to this. Fun. (Linux
2002111605)

Comment 27

[Pierre Chanial]

Jeremy, please calm down and see my last comment.
This bug does not appear in 1.1 and will be fixed for 1.2. Nightlies/alpha/beta
are at the user own risks.

Comment 28

[Robert John Churchill]

Bumping back to chanial... reassign to Ben if you don't have time to fix.

chanial:  It appears that you don't fully understand RDF aggregation (who does,
really?), so please send me email and we can go over the details... including
how some of your changes over the past need to be re-done.

Comment 29

[Pierre Chanial]

rjc: I'll be very happy to discuss with you about aggregation, thanks! I am
still stand with the code I introduced to deal with bookmark resources not
hooked in the graph, but let's discuss it via E-mail (I am *really* busy these
days, so I expect this dicussion to happen not in a short term)
Anyway, I have to stress that I am not responsible for this regression nor any
part of the 'paste' functionality in the mozilla code.
I suggest to back out the patch in bug 145858 and reopen it until we come with a
correct patch that 1) allow to access unhooked bookmark resources and 2) deals
with aggregation

patch will follow

Comment 30

[Pierre Chanial]

Attachment: patch v1.0

this patch reverts to the situation we were for Moz 1.1
It is untested and it's the only contribution I can do for 1.2
sergei: I dropped the #ifdef BeOS also, since cut/paste would also fail.

Ben, if you can come with a better patch, feel free to obsolete this one and
reassign it to you

Comment 31

[Robert John Churchill]

Undoing the changes causes other bugs, BTW.  Ben wrote the "paste" method in
bookmarks.xml so I suspect he can come up with a more appropriate fix fairly
easily that doesn't cause other regressions.

Comment 32

[Sergei Dolgov]

pch, as per comment #30.
In such case we should return step more backward, when i replaced that ugly :)
C-slang "?:" with  if-else statement - with JUST SAME meaning/effect, but with
specialBeOS case added. 

(Without *aType = kNC_URL as 3-rd case for BeOS only we'll loose bokmark import
in BeOS again. And what about Cut problems in this BeOS-only-third-case - there
isn't such problem, as those are read-only dynamically imported bookmarks with
recreation each time when you look at it.)

Comment 33

[Robert John Churchill]

I bet the issue is that the "paste" code is trying to get the bookmark type...
instead, the "copy" code could get the bookmark type and encode it on the
clipboard along with the URL.

Since chanial indicates he is too busy, reassigning to Ben

Comment 34

[Sergei Dolgov]

As reminder, code which prevents regression for BeOS was:
    if (isContainer)
    {
      *aType =  kNC_Folder;
    }
	else
	{
#if defined(XP_BEOS)
      //solution for BeOS - bookmarks are stored as file attributes. 
      PRBool isBookmarkedFlag = PR_FALSE;
      rv = IsBookmarkedInternal(aNode, &isBookmarkedFlag);
      if (!isBookmarkedFlag || NS_FAILED(rv) || (rv == NS_RDF_NO_VALUE))
        *aType = kNC_URL;
      else
#endif
       *aType = kNC_Bookmark;	
    }

Comment 35

[Pierre Chanial]

sergei: import would break, but the patch that fixes it breaks cut/paste on
'regular' bookmarks for BeOS also, I guess.

rjc: I don't see why a bookmark resource that is not in a container (a cut
bookmark) is prevented to be modified and why GetSynthesizedType would not
return '...#Bookmark' for it

Comment 36

[Robert John Churchill]

chanial:  RDF datasources should not positively answer queries against resources
that they know nothing about.

Comment 37

[Robert John Churchill]

Attachment: Patch

Here's an example of the sort of fix I'd like to see for this bug.  This simple
patch passes along the type.  (I tested it a tad -- works for cut/copy/paste of
bookmarks, folders, separators, etc.)

Comment 38

[Asa Dotzler [:asa]]

Without this patch from rjc I get this error in the console when I try to paste:
###!!! ASSERTION: null ptr: 'aURI != nsnull', file nsRDFService.cpp, line 994
Break: at file nsRDFService.cpp, line 994
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80004003
(NS_ERROR_INVALID_POINTER) [nsIRDFService.GetResource]"  nsresult: "0x80004003
(NS_ERROR_INVALID_POINTER)"  location: "JS frame ::
chrome://communicator/content/bookmarks/bookmarks.xml#bookmarks-tree.paste() ::
paste :: line 62"  data: no]
************************************************************
An error occurred executing the cmd_bm_paste command

With the patch applied I get no error and the paste succeeds. Myk also tested
the patch on a branch build on linux with the same positive results. 

Comment 39

[Asa Dotzler [:asa]]

I've done some preliminary regression testing with good results. I can paste a
bookmarks folder onto the personal toolbar or a bookmark into a folder on the
personal toolbar. I can paste a bookmark into html email. I can paste a bookmark
into a text editor. Copy from bookmarks and paste into all of these locations
works. Tested with a current 1.2 branch debug build on RH 8. 

Comment 40

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

aha!

Thanks for the illuminating error message, Asa. It's helped answer my questions.
(which don't appear to have shown up). 

the segment:
..
if (!rType) {
  const krName = ksRDF.GetResource(names[i]);
..

made an assumption: if there was no type arc on a given node, this code assumed
the bookmark was of flavour text/x-moz-url, i.e. originating from outside the
app. The text/x-moz-url format is url\nname, so it assumed it could index into
the names[] array and find the name part of the data for this bookmark. 

Since Bookmarks switched to using synthesized types, rType was null here, so
this code was entered for all cases, as rjc said. This caused names[i] to be
accessed when it did not exist. 

rjc's patch fixes this bug by ensuring there is always type data for every
bookmark encoded on the clipboard as moz/bookmarkclipboarditem, so once again
this condition is never entered. 

However, it's probably safest to do something like: 

if (!rType && names[i])  {
...
}

instead of just

if (!rType) {
...
}

to prevent future bugs. 

sr=[e-mail] with that change. 

Comment 41

[Asa Dotzler [:asa]]

Comment on attachment 106818 [details] [diff] [review]
patch v1.0

obsoleting pierre's patch (hope I'm not stepping on toes here). pch, can you
review this patch from rjc?

Comment 42

[Pierre Chanial]

I will discuss the issue with rjc later, when I will have less pressure, and
when moz1.2 will be out. I still think there is no need to pass the type in the
clipboard since he bookmark type information do exist in the ds: we just return
empty when the bookmark is not in a container. (the container itself being in
the BookmarksRoot hierarchy or not...)
other consumers of moz/bookmarkclipboarditem should be fixed, too. I guess that
cutting/pasting in the PT, from the PT to the tree and from the tree to the PT
will have problems.
fix that and r=[e-mail]

Comment 43

[Asa Dotzler [:asa]]

rjc, can you help us get this fix landed on the 1.2 tree. Are these reviews
sufficient? Does the fix need additional changes?

Comment 44

[Robert John Churchill]

Landed fix on trunk as well as MOZILLA_1_2_BRANCH

Comment 45

[Asa Dotzler [:asa]]

Comment on attachment 106847 [details] [diff] [review]
Patch

setting flags that were missed.

Comment 46

[[e-mail]]

Comment on attachment 106847 [details] [diff] [review]
Patch

Also gets rid of those nasty try/catches. Neat!