Closed Bug 196942 Opened 17 years ago Closed 13 years ago

Ability to replicate server events to local for offline viewing

Categories

(Calendar :: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

VERIFIED DUPLICATE of bug 380060

People

(Reporter: grant.fengstad, Assigned: jminta)

Details

(Whiteboard: [high risk][no l10n impact])

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030304
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030304

It would be very useful for future consideration that as the Mozilla Calendar
begins to support servers (ie: ICAL based, Sun One, Sun CDE, etc) that the
client have an ability to replicate and keep a local copy of the calendar data
for offline viewing.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Mike and his group are doing work on a general calendar server interface, I
don't know if this applies to them or not.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I agree that this would be usefull. I wouldn't like the idea of being able to
edit the offline file, but viewing it would be very helpful. If we let people
edit it, we'd have to deal with merging changes and all that.
Component: Calendar Front End → Calendar General
Depends on: 188660
OS: Windows XP → All
Summary: Enhancement Request: Ability to replicate server events to local for offline viewing → Ability to replicate server events to local for offline viewing
New contact from mikep@oeone.com to mostafah@oeone.com
Filter on string OttawaMBA to get rid of these messages. 
Sorry for the spam.
Assignee: mikep → mostafah
QA Contact: gurganbl → general
Flags: blocking0.3?
Reassigning all automatically assigned bugs from Mostafa to nobody@m.o

Bugspam filter: TorontoMostafaMove
Assignee: mostafah → nobody
This is going to go on the blocking radar as its an 0.2 regression.  We can mark the cal read-only for 0.3, to avoid the sync problems.
No longer depends on: 188660
Flags: blocking0.3? → blocking0.3+
Whiteboard: [swag:3d]
Attached patch work in progress (obsolete) — Splinter Review
Patch is an untested work-in-progress.  I'm putting it here for feedback on the general architecture I'm using here (re-using backup files).  Opinions?
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Whiteboard: [swag:3d] → [swag:3d][high risk]
Attached patch use backup for offline v1 (obsolete) — Splinter Review
Full implementation of backup-as-offline source.  Kept small for 0.3 eligibility.
Attachment #232654 - Attachment is obsolete: true
Attachment #233691 - Flags: second-review?(dmose)
Attachment #233691 - Flags: first-review?(mvl)
Whiteboard: [swag:3d][high risk] → [swag:3d][high risk][patch in hand]
Comment on attachment 233691 [details] [diff] [review]
use backup for offline v1

         } catch(e) {
             // File not found: a new calendar. No problem.
+            this.tryUsingBackup();
             this.unlock();
This comment needs to change slightly now.

+            if (!this.mUsingBackup) {
+                this.tryUsingBackup();
+            } else {
+                this.mUsingBackup = false;
+            }
Swap these cases and add a comment.

+        var latestFile = null;
+        while (dirEnum.hasMoreElements()) {
+            var file = dirEnum.getNext().QueryInterface(CI.nsIFile);
+            if (file.isFile()) {
+                if (!latestFile || file.lastModifiedTime > latestFile.lastModifiedTime) {
+                    latestFile = file;
+                }
+            }
+        }
You need to make sure you have the right backup file for this calendar by using the unique id.
Attachment #233691 - Flags: second-review?(dmose)
Attachment #233691 - Flags: second-review-
Attachment #233691 - Flags: first-review?(mvl)
Patch addresses previous review comments.  The minor tweaks have been incorporated and we're now looking at the id of the file/calendar to make sure they match.
Attachment #233691 - Attachment is obsolete: true
Attachment #234067 - Flags: second-review?(dmose)
Attachment #234067 - Flags: first-review?(mvl)
Comment on attachment 234067 [details] [diff] [review]
use backup for offline v2

     locked: false,
 
+    mUsingBackup: false,
+
Comment here explaining what mUsingBackup is.

     get readOnly() { 
-        return this.mReadOnly;
+        return this.mReadOnly || this.mUsingBackup;
Comment here why mUsingBackup matters here.

+            var file = dirEnum.getNext().QueryInterface(CI.nsIFile);
+            if (file.isFile() && file.leafName.indexOf(prefix) != -1) {
+                if (!latestFile || file.lastModifiedTime > latestFile.lastModifiedTime) {
can use substr here instead of indexof.

r=dmose with that.
Attachment #234067 - Flags: second-review?(dmose) → second-review+
I'm sorry to say, but i'm not happy with this approach.
We already had other plans, with offline data in a storage cal, and using that for sync etc. Why switch to something else now, and use something that was not made for this? Adding this read-only offline code does seem like the way to go. I'm afraid it will bother us later, because we can't just remove the feature.

The other problem is with the user experience. All kind of magic things seem to happen. Suddenly your calendar is read-only, without any message as to why and how to change that to read-write again.
(In reply to comment #11)
> I'm sorry to say, but i'm not happy with this approach.
> We already had other plans, with offline data in a storage cal, and using that
> for sync etc. Why switch to something else now, and use something that was not
> made for this? Adding this read-only offline code does seem like the way to go.
> I'm afraid it will bother us later, because we can't just remove the feature.
We can change the model without removing the feature.  The idea here was to provide something usable for 0.3 with minimal amount of code/review time.  Actually beginning the storage path requires much more planning/architecture that I don't think could be done in time for 0.3.  The patch is quite self-contained, and can easily be removed when something more permanent comes along.

> 
> The other problem is with the user experience. All kind of magic things seem to
> happen. Suddenly your calendar is read-only, without any message as to why and
> how to change that to read-write again.
> 
I think that the user would be generally aware of the fact that they're now offline.  If you really want a prompt to come up, that's certainly possible to do.
Whiteboard: [swag:3d][high risk][patch in hand] → [high risk][patch in hand]
Whiteboard: [high risk][patch in hand] → [high risk][patch in hand][needs review mvl]
Whiteboard: [high risk][patch in hand][needs review mvl] → [high risk][patch in hand][needs review mvl][no l10n impact]
Comment on attachment 234067 [details] [diff] [review]
use backup for offline v2

>Index: calendar/providers/ics/calICSCalendar.js
>     set uri(aUri) {
>-        if (channel instanceof Components.interfaces.nsIHttpChannel) {
>-            this.mHooks = new httpHooks();

Don't move this code. hooks might keep state, and that likely needs to be kept for each reload. So only construct the hooks once.

>     refresh: function() {
>+        // Give ourselves another chance to download the file correctly
>+        this.mUsingBackup = false;

This really wants more documentation. One block of comments, describing the backup and using the backup would be good.

>         var streamLoader = Components.classes["@mozilla.org/network/stream-loader;1"]
>                                      .createInstance(Components.interfaces.nsIStreamLoader);
>         try {
>             streamLoader.init(channel, this, this);
>         } catch(e) {
>-            // File not found: a new calendar. No problem.
>+            // For a new calendar, this just means file-not-found, which
>+            // isn't a problem.  We won't have any backups either, so its
>+            // not a big deal.  This may, however, mean that an existing
>+            // calendar is not reachable, so we should try to use the backup.

There are better ways to guess at what really happened, like looking at the http error codes. This kind of guessing might lead to problems.

>+            this.tryUsingBackup();

>     /**
>+     * For the case where we previously had a remote calendar, but now we're
>+     * offline, we can use the backup (which was the last copy of the calendar
>+     * that the user saw anyway) to still show the items.  We'll mark this

That's not true. It is the last version before the last change was uploaded. So you always miss one change.
Actually, thinking about it, that might be a source of dataloss. Need to test.

>+     * calendar read-only for now (via mUsingBackup) in order to avoid the whole
>+     * syncing mess.

Syncing is not a mess, it is just not written yet.


>+    tryUsingBackup: function ics_tryBackup() {

>+        try {
>+            var dirService = Components.classes["@mozilla.org/file/directory_service;1"]
>+                                       .getService(CI.nsIProperties);
>+            var backupDir = dirService.get("ProfD", CI.nsILocalFile);
>+            backupDir.append("backupData");
>+            if (!backupDir.exists()) {
>+                return;
>+            }
>+        } catch(e) {
>+            // Backup dir wasn't found. Likely because we are running in
>+            // xpcshell.
>+            return;
>+        }

I smell code duplication!

>+            streamLoader.init(channel, this, this);

Calling the streamloader from a callback of the streamloader makes my head hurt...
This isn't going to make it in by tonight, and the fact that it's one version back as pointed out by mvl is essentially dataloss for the user.  Let's hold off on this until post-0.3.
Flags: blocking0.3+ → blocking0.3-
Comment on attachment 234067 [details] [diff] [review]
use backup for offline v2

r-, based on the loss of the last edit.
Attachment #234067 - Flags: first-review?(mvl) → first-review-
I'm sorry, hope this is not concidered as spam (tried to post in the mozilla calendar-forum, but no luck). But does "post0.3" mean that this bug will not get fixed in the 0.3 version? I can only speak for myself, but this feature is one of the most important why I wait for the 0.3 version.
Thanks anyway for your efforts in improving sunbird/lightning.
Whiteboard: [high risk][patch in hand][needs review mvl][no l10n impact] → [high risk][no l10n impact]
Is it possible to make this patch an add-on? 
Blocks: 371300
Duplicate of this bug: 366787
see also bug 361955#2
This is to encourage those working on this enhancement!  I have specified SunBird for our company's Windows platforms to do very simple collaborative calendaring over an internal LAN.  To view the calendars (read-only) I use iCal on my Mac laptop.  No problem for me when the server goes down unexpectedly or I travel, but Windows users lose.  Internal support is miniscule so I can't recommend extra plugins and extra user steps.

Just a report from the user trenches.  Thanks for your great work!  
work in progress bug 380060
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 380060
No longer blocks: 371300
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.