Closed Bug 682109 Opened 13 years ago Closed 10 years ago

Today pane: Today's date not automatically updated

Categories

(Calendar :: Lightning Only, defect)

Lightning 1.0b5
x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mikko.vestola, Assigned: darktrojan)

References

Details

Attachments

(2 files, 2 obsolete files)

I installed the latest version of Thunderbird and installed the latest version of Lightning extension. The problem is that the "Today Pane", which should show the events for today and tomorrow, is not updated automatically. I assume it should change the date automatically to today's date, however, in my case it does not. The date in today pane is only updated when I restart Thuderbird or manually click the "go to today" button (the circle button in the Today pane).

For example, yesterday (2011-08-25) the Today pane always showed the date 2011-08-24. It did not change automatically to 2011-08-25 (or not even to 2011-08-26). I had to manually press the "Go to today" button to change the date.

I just checked that the date in today pane did not change when my computer clock changed from 23:59 to 00:00 (and not even when I waited 15 minutes or so), so I don't think this is an issue with that my laptop is usually as suspended state when the day actually changes. 

I can reproduce this bug all the time. The today's date never gets updated. This is quite annoying because the Today pane should show a quick view about upcoming events, but it is quite useless if I always has to remember to manually press the "Go to today" button.
I can confirm this bug with TB 16.0.2 and Lighting 1.8 on Linux 3.2.0-32, Kubuntu 12.04, and KDE 4.8.5
I can confirm this bug with thunderbird 17.0.8 and lightning 1.9.1 on Linux, if the system is hibernated during date change. On another machine running 24/7 the today pane updates correctly (although I did not check at which time).
This probably has the same root cause as Bug 757762, the today pane needs to refresh when waking from sleep/hibernate, and that notification isn't supported on linux. The real fix would be adding linux support for wake_notification in Bug 758848, but in the mean time maybe we could use a workaround similar to the one for the alarm service.
Depends on: 758848
Attached patch 682109-1.diff (obsolete) — Splinter Review
Got annoyed by this enough to "fix" it.
Assignee: nobody → geoff
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8437364 - Flags: review?(philipp)
Comment on attachment 8437364 [details] [diff] [review]
682109-1.diff

Review of attachment 8437364 [details] [diff] [review]:
-----------------------------------------------------------------

While this will fix the issue, I'd appreciate if you could do a little refactoring. With the code as is, there are two sleep timers. Instead, I think it would be best to create a new XPCOM component only for the sleep monitor. When the timer detects a suspend/hibernate, it should use nsIObserverService's notifyObservers to notify for wake_notification.

With this in place, you can remove the timer in the alarm service, because the wake_notification will shutdown/startup the alarm service. Also, no new code in calendar-views.js is needed because the observer service will notify application wide.

I know you are busy, but I would really appreciate if you had a little time to make these changes :)
Attachment #8437364 - Flags: review?(philipp) → review-
Attached patch 682109-2.diff (obsolete) — Splinter Review
Like this?
Attachment #8437364 - Attachment is obsolete: true
Attachment #8441209 - Flags: review?(philipp)
I've just remembered that I didn't fix it to not run on Windows or OS X. I was going to use manifest flags but that's probably not the *right* way. I guess a check in the observe function is better.
I've tested 682109-1.diff and it works as it should. So why not integrate that one in the next release as a temporary fix and then you would have all the time it needs to find the *right* way to fix it. IMO this opportunity is too good (and you missed already one: 2.6.6) to keep this open for another couple of months, years (?) and I really would prefer not to have to patch all intermediate versions.
Comment on attachment 8441209 [details] [diff] [review]
682109-2.diff

Review of attachment 8441209 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp with the following changes. I'd appreciate if you could upload a new patch for checkin. How confident are you with this patch? The next major release, Lightning 3.3 is but a few weeks away and I would like to keep the regression risk low. If you say this is well tested and working nicely we could on the other hand cross our fingers and push it to beta.

::: calendar/base/content/calendar-views.js
@@ +415,4 @@
>          gMidnightTimer = Components.classes["@mozilla.org/timer;1"]
>                                     .createInstance(Components.interfaces.nsITimer);
>      } else {
> +        cal.LOG("[scheduleMidnightUpdate] Midnight timer recycled.");

Do you think we need these logging messages pushed? I'm sure they were helpful for debugging though :-)

::: calendar/base/src/calAlarmService.js
@@ +129,5 @@
>       */
>      observe: function cAS_observe(aSubject, aTopic, aData) {
>          // This will also be called on app-startup, but nothing is done yet, to
>          // prevent unwanted dialogs etc. See bug 325476 and 413296
> +        cal.LOG("[calAlarmService] observed " + aTopic);

Same here, this debug can probably go.

::: calendar/base/src/calSleepMonitor.js
@@ +1,3 @@
> +const Cc = Components.classes;
> +const Ci = Components.interfaces;
> +const Cu = Components.utils;

File is missing license header. Also, we haven't been shorting to Cc/Ci/Cu in calendar, but if you really want to do this I'd suggest using the "new syntax":

let { Ci: interfaces, Cc: classes, Cu: utils } = Components;

Also, as you've already mentioned, you should only start this monitor on Linux.

@@ +12,5 @@
> +calSleepMonitor.prototype = {
> +    classDescription: "Calendar Sleep Monitor",
> +    classID: Components.ID("9b987a8d-c2ef-4cb9-9602-1261b4b2f6fa"),
> +    contractID: "@mozilla.org/calendar/sleep-monitor;1",
> +    QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver]),

Since you have classDescription and friends, you should also include classinfo, similar to how we do it here: http://mxr.mozilla.org/comm-central/source/calendar/base/backend/calBackendLoader.js#17
Attachment #8441209 - Flags: review?(philipp) → review+
> > +const Cc = Components.classes;
> > +const Ci = Components.interfaces;
> > +const Cu = Components.utils;

We had serious problems with this in the past and therefore had to remove Cc/Ci/Cu from Lightning, see e.g. Bug 390842 and Bug 400316. In addition the verbose names increase the readability.
Attached patch 682109-3.diffSplinter Review
Attachment #8441209 - Attachment is obsolete: true
Comment on attachment 8455812 [details] [diff] [review]
682109-3.diff

Just a quick r? on the changes. If you're happy, please check in (my hg access has expired).
Attachment #8455812 - Flags: review?(philipp)
Comment on attachment 8455812 [details] [diff] [review]
682109-3.diff

Review of attachment 8455812 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=philipp
Attachment #8455812 - Flags: review?(philipp) → review+
Comment on attachment 8455812 [details] [diff] [review]
682109-3.diff

After some IRC discussion with Geoff, I think this should be safe enough for aurora/beta, a=me.
Attachment #8455812 - Flags: approval-calendar-beta+
Attachment #8455812 - Flags: approval-calendar-aurora+
Pushed to comm-central changeset 5b67c1564598
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.5
Backported to releases/comm-beta changeset c7cf3ed9ca21
Target Milestone: 3.4 → 3.3
Bug re-appeared in Lightning 4.0.2/Thunderbird 38.2.0/Windows 8.1.
Aldo, please file a new bug report if you can still observe the problem as this one has been closed for over a year.
Flags: needinfo?(acugnini)
Flags: needinfo?(acugnini)
You need to log in before you can comment on or make changes to this bug.