Add support for launching calendar to xremote

RESOLVED FIXED

Status

()

RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: mostafah, Assigned: blizzard)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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.
(Reporter)

Comment 1

15 years ago
Created attachment 150389 [details] [diff] [review]
Proposed fix to add calendar support to xremote
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.
Attachment #150389 - Flags: superreview?(blizzard)
Attachment #150389 - Flags: review+

Comment 3

15 years ago
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.
OS: Linux → All
(Reporter)

Comment 4

14 years ago
Created attachment 173764 [details] [diff] [review]
Proposed fix to add calendar support to remote (Ver 2)

All issues from comment #2 have been addressed. Another review is appreciated.
Attachment #150389 - Attachment is obsolete: true
Attachment #173764 - Flags: review?(shaver)
Comment on attachment 173764 [details] [diff] [review]
Proposed fix to add calendar support to remote (Ver 2)

r+sr=shaver, thanks!
Attachment #173764 - Flags: superreview+
Attachment #173764 - Flags: review?(shaver)
Attachment #173764 - Flags: review+
(Reporter)

Comment 6

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

Removing request on obsolete patch
Attachment #150389 - Flags: superreview?(blizzard)
(Reporter)

Comment 7

14 years ago
Attachment 150389 [details] [diff] checked in.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.