1262373 - Today pane: DST causes missing dates for events in the "Upcoming" section

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Decathlon]

A few users reported this issue on mozillaZine forum.

Steps to reproduce:

- on your OS, temporarily change the system date to the day when the DST occurs in your country, or the previous day, e.g. 27/3/2016 for central Europe;
- start Thunderbird and create events (normal events and all-day events) that occur in the "upcoming" period of time according to Today Pane's settings.

--> the events showed in the "Upcoming" section of the Today Pane have only the time and the date is missing.

Comment 1

[Merike Sell (:merike)]

This is likely related to bug 1130047 where at least some of the failures where caused by similar symptoms. Also Lightning sometimes gets confused what the current time is: bug 1218500.

Comment 2

[Decathlon]

Attachment: patch-v1

(In reply to Merike (:merike) from comment #1)
> This is likely related to bug 1130047 where at least some of the failures
> where caused by similar symptoms.

Yes it seems strongly related.
This is the patch I already had. I want to verify if there are others issues to take into account before asking for review but it fixes the issue.

If you want to verify whether it affects bug 1130047, please do, otherwise I could try this evening with the try-server.

> Also Lightning sometimes gets confused
> what the current time is: bug 1218500.

This seems quite different because of ical.js.

Comment 3

[Decathlon]

I run the test testTodayPane.js locally with the patch applied after changed the system date to 27/3/2016 (DST change in Europe) and the test passed. Without patch the test failed.

Comment 5

[Decathlon]

Attachment: patch-v2

Apart from the DST problem, there were a few other issues caused by the fact that dates showed with the events are enabled by periods' dates.
This patch uses the the duration of the periods instead of dates.

Now, when displaying a single day in the today pane, no date is displayed along with each item. This is better because the date is already displayed (with bold font) in the header of the period.

I have run locally the test testTodayPane.js with different system dates and, as far as I can see, there's no problem with DST change.

Merike, could you review this so you could also take a look to testTodayPane.js test?

Comment 6

[Merike Sell (:merike)]

Comment on attachment 8739765 [details] [diff] [review]
patch-v2

Works and looks good to me. Just one thing:

> let showDate = aPeriod.duration > 1 || aPeriod.multiday;
Is there a case where the two conditions differ that I'm missing? Wouldn't it work just the same with second condition only?

Comment 7

[Decathlon]

(In reply to Merike (:merike) from comment #6)
> Is there a case where the two conditions differ that I'm missing? Wouldn't
> it work just the same with second condition only?

The following code line:
http://mxr.mozilla.org/comm-central/source/calendar/base/content/agenda-listbox.xml#77
made me think that was possible a case with "synthetic.duration" > 1 that is not "soon" ("upcoming").
Looking at the way the periods are set, that case should be impossible, also because I've never seen in the today pane a period with an interval of dates as header. Since that case is not completely clear to me, I decided to add the mentioned condition just for safety.
If you agree I'd prefer to keep it.

Comment 8

[Merike Sell (:merike)]

Ok, let's keep it in case someone decides to add such periods later on. One bug less they might accidentally create :)

Comment 9

[Decathlon]

I moved the mentioned condition to the second argument in the OR statement.

Pushed to comm-central:
https://hg.mozilla.org/comm-central/rev/1bb6fd6acf84