reload remote calendar doesn't work



Sunbird Only
13 years ago
12 years ago


(Reporter: Michiel van Leeuwen (email: mvl+moz@), Assigned: Michiel van Leeuwen (email: mvl+moz@))





(1 attachment, 1 obsolete attachment)

file->reload remote calendars:

JavaScript strict warning: chrome://calendar/content/calendar.xul, line 1:
reference to undefined property
JavaScript error: chrome://calendar/content/calendar.xul, line 1:
gCalendarWindow.calendarManager.refreshAllRemoteCalendars is not a function

Now if reloading remote calendar is useful open for debate. I think it should
happen automaticly every n minutes, except when offline. But then the UI should
be removed.


13 years ago
Keywords: regression

Comment 1

13 years ago
For this, we should add a refresh() method to calICalendar and simply enumerate
over each calendar and call refresh.  It would then be up to each provider to
update its calendar in the best way.

Comment 2

13 years ago
My idea is that the user shouldn't have to worry about reloading. It should
always be automatic. and if a provider can't do it automatic, it should have a
timer and make it look automatic. (that's what i'm planning for remote ics)
This means that the UI can be removed.

But until we have that, a .reload() might do.

I think we'll probably want some explicit reload mechanism.  People who are
working to test their remote calendar setup will often want to see the new
changes _now_, and not in N minutes when the timer fires.  Maybe if we get it
really solidly right, though, we can move into such a brave new world.

But I agree that a user should not _need_ to reload a remote calendar in the
course of normal operation, in which they don't have "out of system" knowledge
about the arrival pending changes to the remote store.  Even so, we will need
some programmatic way to get the providers up-to-date in preparation for an
explicit offline transition or calendar export, so the interface addition seems


13 years ago
Blocks: 298936

Comment 4

13 years ago
Created attachment 188867 [details] [diff] [review]
initial patch v1

Patch has a suggested interface, plus a very simple impl for the ics provider.
This is initial work. In an ideal world, the ics provider only submits
notifications on the changed events. (we need that anyway for a cached ics, so
it will come one day)
Assignee: mostafah → mvl
Attachment #188867 - Flags: first-review?(shaver)
Comment on attachment 188867 [details] [diff] [review]
initial patch v1

>         streamLoader.init(channel, this, this);
>+        return false;
>     },

Why return false here?	refresh is defined as void-return.

I don't see you implementing refresh on the composite calendar, but your
reloadCalendars function seems like it'll call it there, which would do nothing
but throw an exception.

Can you implement it for composite and storage (a no-op), and stub it to throw
NYI for CalDAV?
Attachment #188867 - Flags: first-review?(shaver) → first-review-

Comment 6

13 years ago
Created attachment 190330 [details] [diff] [review]
updated patch

Updated patch. Fixes the review comments.
Attachment #188867 - Attachment is obsolete: true
Attachment #190330 - Flags: first-review?(shaver)
Comment on attachment 190330 [details] [diff] [review]
updated patch

>+    refresh: function() {
>+        for (cal in this.mCalendars) {
>+            try {
>+                this.mCalendars[cal].refresh();
>+            } catch(e) {}
>+        }
>+    },

for each (cal in this.mCalendars) {
    try { cal.refresh(); } catch (e) { }

Attachment #190330 - Flags: first-review?(shaver) → first-review+

Comment 8

13 years ago
Do we really want a refresh() method on calICalendar? I would think it should be
on something like calIRemoteCalendar instead.
I think it's fine there -- it's at worst a no-op, and I think it's better to
have there than on another interface that will be hard to name.  (Is the
composite a remote one?)

I regret that we split that stuff in RDF land, so long ago.

Comment 10

13 years ago
checked in
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 11

13 years ago
-  <command id="reload_remote_calendars"
+  <command id="reload_remote_calendars" oncommand="reloadCalendars()"/>

Please don't forget about changing Sunbird's commands too...(I don't really feel
like re-opening bug 298772 as well, so maybe just fix them both here, please?)
Resolution: FIXED → ---

Comment 12

13 years ago
ok, i updated sunbird
Last Resolved: 13 years ago13 years ago
Resolution: --- → FIXED
The bugspam monkeys have been set free and are feeding on Calendar :: Sunbird Only. Be afraid for your sanity!
QA Contact: gurganbl → sunbird
You need to log in before you can comment on or make changes to this bug.