The patch I will attach proposes a way to simplify (a lot) the bookmark code and make it robust (move/delete/cut/copy) and maintainable. WHAT: 1) remove the fork code between trees and the personal toolbar. 2) systematic handling of non permitted actions. 3) services defined as const in one place WHY: 1) The current bookmark code is quite complicated and rather difficult to maintain in its current form. All the commands are duplicated in bookmarks.xml, bookmarksOverlay.js and a bit in personalToolbar.js. None of the version is bug-free. 2) Wrong actions have been prohibited patch after patch, (copy a folder into itself, drop in find:bookmarks or file: bookmarks, etc...) some part of the patch is corrected but the major part isn't. 3) There is no naming convention (RDF, RDFC, etc...). I found a misleading naming. But the interest here is to make the code much more readable by not redefining those services everytime we need them. HOW? 1) The commands were defined in BookmarksUIElement.prototype and <bookmarks-tree>. They will be removed in bookmarks.xml and ve only defined in a var called BookmarksCommand. A method specific to trees |getTreeSelection| and another one |getPTSelection| specific to the personal toolbar will return an object containing the selection. This object will be passed to the methods in BookmarksCommand. 2) the selection will contain information whether the selected item is immutable or not. The immutability will be computed once in one place. It will also contain the item parent URI so that the bookmarks will be uniquely identified. A method |checkTargetFolder| will check if the selection can be dropped (paste, moved...) in a folder. CM menu, buttons and commands will use this information to allow or forbid the actions. 3) services, bookmark datasource will be defined in one place at the top of bookmarksOverlay.js. A better place would obviously be a proper file services.js but due to redefinition conflicts, I prefer to postpone this issue. The patch: It implements only one command in the way I described previously: File bookmarks renamed as Move Bookmarks (bug 159762) Note: The File Bookmarks command in the Bookmark menu in the PT or menubar is unrelated to this one and is therefore not touched. Only the bookmark manager and the CM are affected by the name changing. Since the patch is just a first step, BookmarksCommand is still assigned to BookmarksUIElement.prototype. In addition, CM does not use yet the facilities provided by the selection to disable its menuitems.

note: this patch does not break the mozilla bookmark handling, even if it only deals with |Move Bookmarks| Comments are welcome!!!

Patch addressing timeless review

Well, here is the last version of my patch. I'll give some precision tomorrow

So far, this patch needs still a bit of work, but I would like to gather some feedback before polishing it. Basically what it does it: - gets rid of the duplicated RDF projections (tree and DOM) and do the work with RDF, only. To do so, I implemented two methods getTreeSelection and getBTSelection (BT stands for bookmark toolbar) that set up an object 'selection' that does not include tree or DOM representation but instead, include the selected item and parent RDF node. That way, I could remove 1200 lines in bookmarks.xml and completely drop the file personalToolbar.js (500 lines). All the actions are now in BookmarksOverlay.js - this selection object includes pertinent information, that will prevent silly actions, that can be easily tweaked. - actions are performed in only two methods: insertSelection and removeSelection - implements the editor transaction manager for undoing/redoing bookmark actions. So far, drag, delete, new folder|bookmark|separator, move actions are handled. Still need to do the import.

also I xblyfied the personal toolbar. Bookmark can now have two light representations: - tree (bookmarks.xml that should be renamed into bookmarksTree.xml) - toolbar (bookmarksToolbar.xml) There is still the menu to be done, though The motivation is clearly for easier use of the bookmark module by clients The patch removes also the unused files: - bookmarksTree.js - bookmarksDD.js Note: - I know that the interdependencies between bookmarksToolbar.xml and navigatorDD.js still suck but should be adressed when the drop into folder feature will be widgetized. - and I am also aware that I did not include the new file bookmarksToolbar.xml in the proper manner (jar.mn and proper cvs)

Dude, you rock.

moving to m1.3a, unfortunately, I will have no time to update this patch and because I can not guarantee to be on the hook to fix the eventual regressions. This patch will be checked in phoenix, though, quite soon.

That's the right way to go, dont give up! I'll closely watch improvement on bookmark handling hopefully landing in 1.3a Regards,

What is happening with this bug?

Sorry, there are still some issues that I have to hammer first. Currently, I have no time to work on it. ->1.5 (maybe 1.4 hopefully)

let's be optimistic -> 1.4a

I really look forward to see this checked in.

landed in the bookmark branch. should be ready for 1.4b early

The bookmarks branch has landed. Fixed.

This also fixed _at_least_ bug 53422, bug 81895, bug 90567, bug 92317, bug 122199, bug 138523, bug 159762, bug 160019 and bug 165150. Can a bookmark branch person please resolve them?

Was this patch supposed to fix a problem pointed out in comment #20 of bug #90567? Steps to reproduce: 1) open Bookmarks|Manage Bookmarks 2) select and highlight a bookmark contained in a folder 3) click on "File Bookmarks" button 4) select the SAME folder for the target location and click "OK" Result: bookmark not displayed in expanded folder view until it is collapsed and reopened. Expected Result: the bookmark should be displayed without extra effort.

Becki, it works for me with a recent nightly.