enhancements to xremoteservice (add adressbook)




16 years ago
2 years ago


(Reporter: Matthew Luckie, Assigned: blizzard)



Firefox Tracking Flags

(Not tracked)




(1 attachment, 1 obsolete attachment)



16 years ago
mozilla has an option to start with the addressbook on the command line with
mozilla -addressbook, but no associated xfeDoCommand for people who wish to open
their address book with a -remote directive to a running mozilla instance.

A patch is available at http://moat.nlanr.net/~mjl/patch-xremoteservice that
adds -remote xfeDoCommand(openAddressBook) functionality.

It also ensures that when xfeDoCommand(openInbox) or xfe(openBrowser) is used,
the window is created with resize bars on it.  I consider these two enhancements
to be bug fixes.

Comment 1

16 years ago
the unresizable scrollbars only appears with some windowmanagers.  "resizable"
is included in "all", but "all" is ignored because of bug 117114.  the patch in
bug 149126 also fixes this

please create an attachment for your patch

==> XRemote
Assignee: racham → blizzard
Component: Address Book → X-remote
Product: MailNews → Browser
QA Contact: nbaca → blizzard

Comment 2

16 years ago
Created attachment 102701 [details] [diff] [review]
adds support for xfeDoCommand(openAddressBook)

Comment 3

15 years ago
confirming as RFE
Ever confirmed: true
Summary: enhancements to xremoteservice → enhancements to xremoteservice (add adressbook)

Comment 4

15 years ago
Comment on attachment 102701 [details] [diff] [review]
adds support for xfeDoCommand(openAddressBook)

with bug 149126 fixed, the "resizable" fixes are no longer needed.
Attachment #102701 - Flags: review?(blizzard)


14 years ago
Attachment #102701 - Flags: review?(blizzard) → review+


14 years ago
Attachment #102701 - Flags: superreview?(dmose)

Comment 5

14 years ago
Comment on attachment 102701 [details] [diff] [review]
adds support for xfeDoCommand(openAddressBook)

Just a few minor nits:

>diff -uBwr src.old/XRemoteService.cpp src/XRemoteService.cpp
>--- xpfe/components/xremote/src/XRemoteService.cpp.orig	Fri Aug  2 11:12:36 2002
>+++ xpfw/components/xremote/src/XRemoteService.cpp	Sun Oct 13 11:16:58 2002
>@@ -566,6 +566,13 @@
> }
> nsresult
>+XRemoteService::GetAddressBookLocation(char **_retval)
>+  *_retval = nsCRT::strdup("chrome://messenger/content/addressbook/addressbook.xul");
>+  return NS_OK;

Since we no longer have to worry about Mac OS 9, we can ditch
nsCRT::strdup(const char *).  Just use strdup() here instead; many C libraries
have hand-tuned the assembly for this sort of function, and that's performance
nsCRT::strdup won't live up to.

> XRemoteService::GetMailLocation(char **_retval)
> {
>   // get the mail chrome URL
>@@ -837,7 +844,7 @@
>       nsCOMPtr<nsIDOMWindow> newWindow;
>-      rv = OpenChromeWindow(0, mailLocation, "chrome,all,dialog=no",
>+      rv = OpenChromeWindow(0, mailLocation, "resizable,chrome,all,dialog=no",
> 			    nsnull, getter_AddRefs(newWindow));
>     }
>   }
>@@ -850,7 +857,7 @@
>       return NS_ERROR_FAILURE;
>     nsCOMPtr<nsIDOMWindow> newWindow;
>-    rv = OpenChromeWindow(0, browserLocation, "chrome,all,dialog=no",
>+    rv = OpenChromeWindow(0, browserLocation, "resizable,chrome,all,dialog=no",
> 			  nsnull, getter_AddRefs(newWindow));
>   }
>@@ -858,6 +865,28 @@
>   else if (aArgument.EqualsIgnoreCase("composemessage")) {
>     nsCString tempString("mailto:");
>     rv = OpenURL(tempString, nsnull, PR_FALSE);
>+  }
>+  // open the address book window
>+  else if (aArgument.EqualsIgnoreCase("openaddressbook")) {
>+    nsCOMPtr<nsIDOMWindowInternal> domWindow;
>+    rv = FindWindow(NS_LITERAL_STRING("mail:addressbook").get(),
>+		    getter_AddRefs(domWindow));
>+    if (NS_FAILED(rv)) return rv;

Please put the if and return on separate lines.  Otherwise it's not possible to
set a breakpoint on the return in most debuggers.

>+    if (domWindow) domWindow->Focus();
>+    else {
>+      nsXPIDLCString abLocation;
>+      GetAddressBookLocation(getter_Copies(abLocation));
>+      if (!abLocation)	return NS_ERROR_FAILURE;

Looks like there's a tab character here instead of an EOL.  Please fix.

With the above changes, sr=dmose.
Attachment #102701 - Flags: superreview?(dmose) → superreview+

Comment 6

14 years ago
>+      nsXPIDLCString abLocation;
>+      GetAddressBookLocation(getter_Copies(abLocation));

don't do this. even if you use strdup() like dmose suggests, you are mismatching
the allocators (nsXPIDLString assumes the allocator that ToNewString() and
friends use; this is _not_ the same as malloc/free!).

why do you need to allocate the string anyway? just define a static string in
the global scope (eg at the top of the file):

static const char kAddressBookLocation[] =

and pass that directly into OpenChromeWindow(). that'll work fine, no?

Comment 7

14 years ago
Created attachment 135433 [details] [diff] [review]
adds support for xfeDoCommand(openAddressBook)

this is a revision of a previous patch that includes suggestions made by others
in the bugzilla system on said patch.
Attachment #102701 - Attachment is obsolete: true


14 years ago
Attachment #135433 - Flags: review+

Comment 8

14 years ago
*** Bug 241761 has been marked as a duplicate of this bug. ***

Comment 9

14 years ago
There's another patch in the dup: attachment 147062 [details] [diff] [review]

Comment 10

2 years ago
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.
Last Resolved: 2 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.