246115 - Add support for launching calendar to xremote

Status: RESOLVED FIXED

(Core Graveyard :: X-remote, defect)

Description

[Mostafa Hosseini]

This is a feature requested by many of the calendar users.
They want to be able to launch the calendar from command line while
mozilla/firefox/thunderbird is already running.
The change is easy but it depends on whether we want this in or not.

Comment 1

[Mostafa Hosseini]

Attachment: Proposed fix to add calendar support to xremote

Comment 2

[Mike Shaver (:shaver emeritus)]

Comment on attachment 150389 [details] [diff] [review]
Proposed fix to add calendar support to xremote

>+nsresult
>+XRemoteService::GetCalendarLocation(const char **_retval)
>+{
>+  // get the Compose chrome URL
>+  *_retval = "chrome://calendar/content/calendar.xul";

Should this be based on a pref, like GetBrowser and GetMailLocation?

>+  else if (aArgument.EqualsIgnoreCase("opencalendar")) {
>+    const char * calendarLocation;
>+    rv = GetCalendarLocation(&calendarLocation);
>+    if ( rv != NS_OK)

Bonus whitespace should be removed:
 if (rv != NS_OK)
but it seems like |if (NS_FAILED(rv))| would be better still.

>+    nsCOMPtr<nsIDOMWindow> newWindow;
>+    rv = OpenChromeWindow(0, calendarLocation, "chrome,all,dialog=no",
>+                          arg, getter_AddRefs(newWindow));
>+  }

Do we want to try to find an existing window if it's open, like Mail does?

I think the pref basing and window reuse can go in later, I just wanted to
capture them here.  Please fix the NS_FAILED thing before checking in, though.

Comment 3

[Dimitrios]

It would be really useful (imo) if the patch addresses the issue described in
http://bugzilla.mozilla.org/show_bug.cgi?id=219589#c33. Changing OS->All since
the problem is visible on Windows too.

Comment 4

[Mostafa Hosseini]

Attachment: Proposed fix to add calendar support to remote (Ver 2)

All issues from comment #2 have been addressed. Another review is appreciated.

Comment 5

[Mike Shaver (:shaver emeritus)]

Comment on attachment 173764 [details] [diff] [review]
Proposed fix to add calendar support to remote (Ver 2)

r+sr=shaver, thanks!

Comment 6

[Mostafa Hosseini]

Comment on attachment 150389 [details] [diff] [review]
Proposed fix to add calendar support to xremote

Removing request on obsolete patch

Comment 7

[Mostafa Hosseini]

Attachment 150389 [details] [diff] checked in.