cannot drag and drop url (from address bar or webpage) to bookmark toolbar. known bug or regressed from enabling places for history handling? Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a1) Gecko/20061215 Minefield/3.0a1 ID:2006121517 [cairo] WFM Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a1) Gecko/20061212 Minefield/3.0a1 ID:2006121204 [cairo]

(In reply to comment #0) > cannot drag and drop url (from address bar or webpage) to bookmark toolbar. Confirmed WFM Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061215 Minefield/3.0a1 - Build ID: 2006121510 Not working after Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061215 Minefield/3.0a1 - Build ID: 2006121511

Latest trunk/Linux build has also this problem. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20061217 Minefield/3.0a1

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

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

a regression from my history-on-places landing. From http://lxr.mozilla.org/seamonkey/source/browser/components/bookmarks/content/bookmarks.js#1905 var HISTDS = RDF.GetDataSource("rdf:history"); that's not going to work anymore. my apologies for not catching it when testing.

looking at http://lxr.mozilla.org/seamonkey/search?string=rdf%3Ahistory, this is the only one I need to worry about.

Code for the wise: MOZ_PLACES #ifdefs in browser/components/bookmarks (is not that remotely innovative?).

working on the fix. we need to execute a query on the nav history service (nsINavHistoryService) and use the title (from nsINavHistoryResultNode). note, we need to do the same thing for createLivemark() method in the same file.

first draft of patch. not ready for review yet. still to do: 1) test createLivemark() path 2) make sure I didn't break non-places bookmarks 3) double check query options (should it be RESULTS_AS_URI?) 4) test the "no title for uri" code paths (with default name and without) 5) investigate this error when dragging from places-history-sidebar to personal toolbar. JavaScript error: chrome://global/content/nsDragAndDrop.js, line 22: aTransferDa taSet has no properties (I think this might have to do with the places controller. I may spin this bug out.)

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

removed the unused createLivemark() method

this patch fixes the regression and removes the unused code. as for the js error, when dragging from the history-panel-on-places sidebar to bookmarks, I still see this error: JavaScript error: chrome://global/content/nsDragAndDrop.js, line 22: aTransferDataSet has no properties I'm going to spin up a new bug about that, as i think the fix will be to the places controller.js, which we can look into after mano lands his monster patch (see #359462, pending review). note, you do not see that js error when dnd links from bookmarks panel, pages, the url bar, etc.

> I'm going to spin up a new bug about that see bug #364401.

Comment on attachment 249173 [details] [diff] [review] patch alternatively, mano could review this.

Comment on attachment 249173 [details] [diff] [review] patch >Index: browser/components/bookmarks/content/bookmarks.js >=================================================================== >RCS file: /cvsroot/mozilla/browser/components/bookmarks/content/bookmarks.js,v >retrieving revision 1.127 >diff -u -8 -p -r1.127 bookmarks.js >--- browser/components/bookmarks/content/bookmarks.js 10 Dec 2006 22:40:39 -0000 1.127 >+++ browser/components/bookmarks/content/bookmarks.js 19 Dec 2006 23:55:36 -0000 >@@ -1892,51 +1892,69 @@ var BookmarksUtils = { > var selection = {}; > selection.length = 1; > selection.item = [aItem ]; > selection.parent = [aParent]; > this.checkSelection(selection); > return selection; > }, > >- createBookmark: function (aName, aURL, aCharSet, aDefaultName) >+ getTitleForURLFromHistory: function(aURL, aDefaultName) > { >- if (!aName) { >- // look up in the history ds to retrieve the name >- var rSource = RDF.GetResource(aURL); >- var HISTDS = RDF.GetDataSource("rdf:history"); >- var nameArc = RDF.GetResource(gNC_NS+"Name"); >- var rName = HISTDS.GetTarget(rSource, nameArc, true); >- aName = rName ? rName.QueryInterface(kRDFLITIID).Value : aDefaultName; >- if (!aName) >- aName = aURL; >+ var title = null; >+ >+#ifndef MOZ_PLACES >+ // look up in the history ds to retrieve the name >+ var rSource = RDF.GetResource(aURL); >+ var HISTDS = RDF.GetDataSource("rdf:history"); >+ var nameArc = RDF.GetResource(gNC_NS+"Name"); >+ var rName = HISTDS.GetTarget(rSource, nameArc, true); >+ title = rName ? rName.QueryInterface(kRDFLITIID).Value : aDefaultName; >+#else >+ var histsvc = >+ Components.classes["@mozilla.org/browser/nav-history-service;1"] >+ .getService(Components.interfaces.nsINavHistoryService); >+ >+ // query for the URL >+ var options = histsvc.getNewQueryOptions(); >+ options.resultType = options.RESULTS_AS_URI; >+ var query = histsvc.getNewQuery(); >+ query.uri = IOSVC.newURI(aURL, null, null); >+ var result = histsvc.executeQuery(query, options); >+ var root = result.root; >+ root.containerOpen = true; >+ var cc = root.childCount; >+ for (var i=0; i < cc && !title; ++i) { >+ var node = root.getChild(i); >+ title = node.title; > } >+ >+ if (!title) >+ title = aDefaultName; >+#endif >+ >+ if (!title) >+ title = aURL; >+ >+ return title; >+ }, >+ It would be cleaner to avoid the title variable and return early for each case.

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

Comment on attachment 249173 [details] [diff] [review] patch see comment 15

> It would be cleaner to avoid the title variable and return early for each case. thanks for looking this over, asaf. I'll clean it up, per your comment, and attach a new patch.

Comment on attachment 249793 [details] [diff] [review] updated patch, per asaf's comments r=mano

fixed. thanks for the review, asaf. Checking in browser/components/bookmarks/content/bookmarks.js; /cvsroot/mozilla/browser/components/bookmarks/content/bookmarks.js,v <-- bookm arks.js new revision: 1.128; previous revision: 1.127 done

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a2pre) Gecko/20061227 Minefield/3.0a2pre ID:2006122714 [cairo] verified fixed for : D&D from location bar to bookmarks toolbar D&D a link from a page to bookmarks toolbar also fixes: D&D from location bar to bookmarks menu D&D a link from a page to bookmarks menu I couldn't find a bug for the latter 2, if one doesn't exist the summary should be changed to: cannot drag and drop url from location bar or webpage to Bookmarks Toolbar or Bookmarks menu.