285016 - can't publish a calendar

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Michiel van Leeuwen (email: mvl+moz@)]

when right-clicking on a calendar in the calendars tab, and selecting 'publish
entire calendar' nothing happens beside an error:

JavaScript strict warning: chrome://calendar/content/calendar.js, line 1378:
reference to undefined property
gCalendarWindow.calendarManager.getSelectedCalendarId
JavaScript error: chrome://calendar/content/calendar.js, line 1378:
gCalendarWindow.calendarManager.getSelectedCalendarId is not a function

Comment 1

[Michiel van Leeuwen (email: mvl+moz@)]

expected results: give a dialog with a field for an url, and a go button.
(should be somewhere in the tree already, it just doesn't get opened because of
the errors)

Comment 2

[Michiel van Leeuwen (email: mvl+moz@)]

the 'old' code to publish could simply upload the ics file, because all
calendars were ics files.
In the new world, this isn't true. So there first a copy to an ics calendar
should be made, to create a file that can be uploaded.
So this bug is more than making the errors go away.

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

I forgot to mention that the ics provider can do the upload. All is needed is to
copy the events into an ics calendar that has the right uri set (and a bit of
juggling with the batch mode)

Comment 4

[Stuart Parmenter]

Attachment: v1

This patch should do the right thing here, but the ICS provider doesn't seem to
want to publish the actual calendar events.

Comment 5

[Michiel van Leeuwen (email: mvl+moz@)]

The problem is in the ics provider. this is what happens:
1. the provider is created, and it's uri is set. This makes it try to load the
file async. At this point, there is no observer yet that will make sure changes
are published.
2. The events are added to the provider.
3. Because there is no observer, the file isn't published
4. The async io finishes. all events are removed, and the (empty) file is parsed
5. only now there is an observer added, but it's too late

The fix is to make the ics provider queue events added before the io is ready.

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

I checked in a patch to the ics provider, to prevent what i desribed in comment
5. It works. Except that now when you publish to an existing file, the events
are added.
To fix that, there need to be an addition to calICalendar.idl to tell the
provider to not load the current items. A different solution would be to let go
of the idea to use the provider for publishing, and come up with some new
interface. That interface could also do xml exporting for example.

Comment 7

[Michiel van Leeuwen (email: mvl+moz@)]

Another way to fix this might be to use the ics exporter from bug 298772. That
will overwrite the remote file.

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: use new interfaces

This patch uses the new interfaces for publishing. (Although the mess with the
storage stream made me wonder if manually doing the conversion to a string
wouldn't be easier. But it works now)
It should fix publishing an entire calendar, and publising the selected items.
ftp with password also works, and the progressbar should also animate. Things
left to be done are mostly making the dialog nicer, like adding a working stop
button, and better error messages.

Comment 9

[Joey Minta]

Comment on attachment 190326 [details] [diff] [review]
use new interfaces

>+    var remotePath = ""; // get a remote path as a pref of the calendar
>+    if (remotePath != "" && remotePath != null) {
>+        var publishObject = new Object( );
>+        publishObject.remotePath = remotePath;
>+        args.publishObject = publishObject;
>     }
The only thing I understand about this is that it was copied from the old code.
 To me, the 'if' looks useless, and I never see us getting the remote path as a
pref.

+    switch(icsURL.scheme) {
+	 case 'http':
+	 case 'https':
Is it too much trouble to support webcal: here too? Not critical, but it would
be nice.

+    uploadChannel.setUploadStream(inputStream,
+				   "text/calendar", -1);
+    channel.asyncOpen(publishingListener, aProgressDialog);

If I try to publish to something like file:///home/joey/doesNotExist/test.ics I
get
Error: [Exception... "Component returned failure code: 0x80520012
(NS_ERROR_FILE_NOT_FOUND) [nsIChannel.asyncOpen]"  nsresult: "0x80520012
(NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame ::
chrome://calendar/content/publish.js :: publishItemArray :: line 145"  data:
no]
Source File: chrome://calendar/content/publish.js
Line: 145
which should be caught/reported.

-      return;
-    }
+	 var ch;
+	 var calendarStringBundle =
srGetStrBundle("chrome://calendar/locale/calendar.properties");
Nit: Please give 'ch' a more descriptive name

+	 try {
+	     ch = request.QueryInterface(Components.interfaces.nsIHttpChannel);
+	     dump(ch.requestSucceeded+"\n");
+	 } catch(e) {
Nit: I don't know all the standards about dump, but I'd prefer to see at least
dump("requestSucceded:"+ch.requestSucceeded+"\n"); so that it is more
meaningful to someone who sees it.

+	 if (ch && !ch.requestSucceeded) {
Let's also report "Calendar published successfully!"

+   document.getElementById( "calendar-publishwindow" ).setAttribute(
"ondialogaccept", "closeDialog()" );
Unless we're planning to do something else in closeDialog later, we should
probably just make this self.close()  and remove the function, for simplicity's
sake.

Finally, I don't know if you want to do it in this bug or separately, but
checkURLField should do a better job checking.	Probably just bring in the
checkURL from calendarCreation.js.

Comment 10

[Joey Minta]

It seems that this actually works right now.  I think pav's changes in March
might have been enough:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/calendar/resources/content&command=DIFF_FRAMESET&file=calendar.js&rev2=1.178&rev1=1.177

Were you hoping to do something more with this bug?

Comment 11

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #10)
> Were you hoping to do something more with this bug?

See comment 6. I'm trying to make publishing to an existing file not append
items, but overwrite the file.

Comment 12

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: new patch

This patch fixes half of the reviewq comments. The other half are more UI
changes, and I want this bug for just the backend. A better UI with better
error reporting and everything can be saved for a different bug (and can be
done after 0.3a1)

Comment 13

[Joey Minta]

Comment on attachment 193076 [details] [diff] [review]
new patch

    if( args.publishObject )
    {
+      gPublishObject = args.publishObject;
       document.getElementById( "publish-remotePath-textbox" ).value =
args.publishObject.remotePath;

This puts 'undefined' in the textbox if there isn't a path defined.  It should
probably be put in an 'if'.
else
    {
       //get default values from the prefs
       document.getElementById( "publish-remotePath-textbox" ).value =
opener.getCharPref( opener.gCalendarWindow.calendarPreferences.calendarPref,
"publish.path", "" );
    }
    document.getElementById( "calendar-publishwindow" ).getButton( "accept"
).setAttribute( "label", publishButtonLabel );	 


This else gets you in trouble.

1.) Select an event.
2.) Choose File_>Publish....
3.) Type in an address
4.) Click 'Publish'
Error: gPublishObject has no properties
Source File: chrome://calendar/content/publishDialog.js
Line: 93

It seems to work if you add gPublishObject - new Object(); in the else.

+    if (icsURL.schemeIs('webdav'))
+	 icsURL.scheme = 'http';
+    if (icsURL.schemeIs('webdavs'))
+	 icsURL.scheme = 'https';

My original concern was for users who look at something like icalshare.com and
see 'webcal://' as an address, and then try to publish similarly.  Can you add 
+    if (icsURL.schemeIs('webcal'))
+	 icsURL.scheme = 'http';

r=jminta with those changes

Comment 14

[Stuart Parmenter]

there is publish code in content/base as well that I assumed sunbird was moving
over to as it provides a slightly nicer interface for publishing.. either way,
should probably fix it up as well since its what lightning uses for publish.

Comment 15

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in.
I filed bug 305526 about moving to base/content (comment 14) and bug 305527
about improving the UI.

Comment 16

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Sunbird Only. Be afraid for your sanity!