203975 - xremote doesn't handle the addBookmark command [PATCH]

Status: RESOLVED INCOMPLETE

(Core Graveyard :: X-remote, defect)

Description

[Massimo Dal Zotto]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0) Gecko/20020623 Debian/1.0.0-0.woody.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0) Gecko/20020623 Debian/1.0.0-0.woody.1

The -remote command of Netscape 4.x supports the addBookmark() command
which can be used from shell scripts to add a new bookmark.

The -remote command of Mozilla has code to handle the addBookmark command
but it doesn't work because it simply returns an error code.

My patch, included below, adds code to implement the addBookmark
in Mozilla. The code seems to work fine, but since I'm not a Mozilla
developer my solution could not be the best way to do it. Any better
suggestion is welcome.

The patch should apply to any version of Mozilla.

Note that to fix an existing installation of Mozilla you need to
compile and replace only the libxremoteservice.so library.


Reproducible: Always

Steps to Reproduce:
1. mozilla &
2. mozilla -remote 'addBookmark(http://some.url)'

Actual Results:  
check the bookmarks, http://some.url is not present


Expected Results:  
should have added http://some.url to my bookmarks


The following patch fixes adds the missing functionality.

--- mozilla/xpfe/components/xremote/src/XRemoteService.cpp.orig	Fri May 17 01:57:51 2002
+++ mozilla/xpfe/components/xremote/src/XRemoteService.cpp	Wed Apr 30 22:02:24 2003
@@ -16,6 +16,9 @@
  * are Copyright (C) Christopher Blizzard.  All Rights Reserved.
  * 
  * Contributor(s):
+ *
+ *  2003-04-30 added code for addBookmark() command
+ *             Massimo Dal Zotto <[e-mail]>
  */
 
 #include "XRemoteService.h"
@@ -47,6 +50,14 @@
 #include <nsIWindowMediator.h>
 #include <rdf.h>
 
+/* For addBookmark() command -- dz */
+#include "nsRDFCID.h"
+#include "nsRDFResource.h"
+#include "nsIRDFService.h"
+#include "nsIRDFDataSource.h"
+#include "nsIBookmarksService.h"
+static NS_DEFINE_CID(kRDFServiceCID, NS_RDFSERVICE_CID);
+
 NS_DEFINE_CID(kWindowCID, NS_WINDOW_CID);
 
 // protocol strings
@@ -257,15 +268,32 @@
       rv = NS_ERROR_NOT_IMPLEMENTED;
     }
     else {
+      char *uri;
+      PRUnichar *name;
+      nsIRDFService* gRDFService = nsnull;
+      nsCOMPtr<nsIRDFDataSource> datasource = nsnull;
+
       index = 0;
       FindLastInList(argument, lastArgument, &index);
       if (!lastArgument.IsEmpty()) {
 	nsCString title(lastArgument);
 	argument.Truncate(index);
-
rv = NS_ERROR_NOT_IMPLEMENTED;
+
name = ToNewUnicode(title);
+      } else {
+
name = ToNewUnicode(argument);
       }
-      else {
-
rv = NS_ERROR_NOT_IMPLEMENTED;
+      uri = argument.get();
+
+      rv = nsServiceManager::GetService(kRDFServiceCID,
+
				NS_GET_IID(nsIRDFService), 
+
				(nsISupports**) &gRDFService);
+      if (NS_SUCCEEDED(rv = gRDFService->GetDataSource("rdf:bookmarks",
+
				 getter_AddRefs(datasource)))) {
+        nsCOMPtr<nsIBookmarksService> bookmarks(do_QueryInterface(datasource));
+
if (bookmarks) {
+
  rv = bookmarks->AddBookmarkImmediately(uri, name, nsnull, nsnull);
+
}
+
nsMemory::Free(name);
       }
     }
   }
--- mozilla/xpfe/components/xremote/src/Makefile.in.orig	Wed Apr 10 05:44:12 2002
+++ mozilla/xpfe/components/xremote/src/Makefile.in	Wed Apr 30 21:49:21 2003
@@ -42,6 +42,8 @@
 
	  necko \
 
	  appshell \
 
	  rdf \
+
	  appcomps \
+
	  rdfutil \
 
	  $(NULL)
 
 CPPSRCS
	= \

Comment 1

[Massimo Dal Zotto]

Attachment: implements the addBookmark xremote command

Comment 2

[Boris Zbarsky [:bzbarsky]]

Chris, any chance you could actually take a look at that patch before that
review request gets to be over a year old?

If you don't have time to, please tell me and I'll look for someone else to
review this code...

Comment 3

[Massimo Dal Zotto]

(In reply to comment #2)
> Chris, any chance you could actually take a look at that patch before that
> review request gets to be over a year old?
> 
> If you don't have time to, please tell me and I'll look for someone else to
> review this code...

Any chance it is fixed before it becomes *TWO* years old?

Comment 4

[Christopher Blizzard (:blizzard)]

Over to vlad who has forgotten more about bookmarks than I've ever known.

Comment 5

[Benjamin Smedberg]

How are we handling app-specific dependencies here?

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 122135 [details] [diff] [review]
implements the addBookmark xremote command

Is this intended for firefox as well?  Firefox's bookmarks implementation stubs
out AddBookmarkImmediately (just returns NS_OK), and it's marked for removal
"RSN" -- though I don't know why, it was before my time.

The "right" way is to obtain a RDF Resource for "NC:BookmarksRoot" and use
bookmarkService->CreateBookmarkInContainer (with aIndex set to -1).

Also, you should probably do_GetService for NS_BOOKMARKS_SERVICE_CONTRACTID
directly instead of going through the rdf data source.

Comment 7

[Benjamin Smedberg]

Mass resolving a bunch of old bugs in the x-remote component in preparation for archiving it. If this bug is still valid and useful, please move it to the "Toolkit: Startup and Profile System" component and reopen it.