1479628 - TodayPane mini-day shows the wrong day of the week

Status: RESOLVED FIXED

(Calendar :: General, enhancement)

Description

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

I thought I was the one that didn't know what day of the week it was, but the today pane is telling me it's Monday, and it's clearly not Monday.

Comment 1

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

Attachment: 1479628-today-pane-weekday-1.diff

The weekday is in a <deck>. (Why? No idea.) Our code sets the right selectedIndex on the deck, and then for some reason it goes back to 1. I *think* this has something to do with the de-XBL-ification of <deck>.

This patch stops us using deck and does something more sensible.

Comment 2

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 8996152 [details] [diff] [review]
1479628-today-pane-weekday-1.diff

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

The weekday was in a deck so it would assume the length of the longest weekday, regardless of the locale. Can you make sure the new code does this as well?

Comment 3

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

Attachment: 1479628-today-pane-weekday-2.diff

After further investigating, I think the weirdness in <deck> has to do with when it is initialised. This is coming to the surface because of the overlay change. If we push setDay to the end of the event queue, the initialisation is properly complete before we set which panel we want.

Comment 4

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 8996589 [details] [diff] [review]
1479628-today-pane-weekday-2.diff

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

::: calendar/base/content/today-pane.js
@@ +40,2 @@
>          TodayPane.setShortWeekdays();
> +        setTimeout(() => TodayPane.setDay(cal.dtz.now()), 0);

Can you add a comment why we are doing this? Otherwise we'll most certainly forget :)

Comment 5

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

Attachment: 1479628-today-pane-weekday-3.diff

That should do it. Should.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2c2d3fef5935
Fix initialisation of TodayPane mini-day, to show the right day of the week. r=philipp DONTBUILD

Comment 7

[[:MakeMyDay]]

This patch seems to cause a js errors - appears for me directly when starting TB - with this patch there's no such error:

<TypeError: aPeriod.start is undefined[Learn More] agenda-listbox.xml:213:17
>    setOccurrence chrome://calendar/content/agenda-listbox.xml:213:17
>    agendaListbox.addItemBefore chrome://calendar/content/agenda-listbox.js:298:5
>    agendaListbox.addItem chrome://calendar/content/agenda-listbox.js:347:35
>    onGetResult chrome://calendar/content/agenda-listbox.js:634:17
>    onGetResult file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calCompositeCalendar.js:532:9
>    queueItems file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calStorageCalendar.js:754:17
>    handleResultItem file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calStorageCalendar.js:785:13
>    getItems_ file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calStorageCalendar.js:846:26
>    getItems/< file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calStorageCalendar.js:665:13
>    postPone resource://calendar/modules/calUtils.jsm:212:13
>    getItems file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calStorageCalendar.js:664:9
>    getItems file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calCompositeCalendar.js:399:41
>    execute chrome://calendar/content/agenda-listbox.js:684:29
>    agendaListbox.refreshCalendarQuery chrome://calendar/content/agenda-listbox.js:693:5
>    agendaListbox.calendarObserver.onLoad chrome://calendar/content/agenda-listbox.js:899:5
>    notify resource://calendar/modules/utils/calDataUtils.jsm:68:32
>    notify resource://calendar/modules/utils/calDataUtils.jsm:96:16
>    onLoad file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calCompositeCalendar.js:44:13
>    notify resource://calendar/modules/utils/calDataUtils.jsm:68:32
>    notify resource://calendar/modules/utils/calDataUtils.jsm:96:16
>    onLoad file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calICSCalendar.js:835:9
>    onStreamComplete file:///F:/workspace/mozilla/thunderbird/obj-x86_64-pc-mingw32/dist/bin/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/components/calICSCalendar.js:222:13

Comment 8

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

Attachment: 1479628-today-pane-weekday-4.diff

I haven't seen that, but I'm not totally surprised. One of these days I'm going to tear the today pane/agenda code apart in frustration and rebuild it.

This should fix it.

Comment 9

[Geoff Lankow (:darktrojan) [OOO, back 5 Jan]]

(In reply to Geoff Lankow (:darktrojan) from comment #8)
> Created attachment 8997213 [details] [diff] [review]
> 1479628-today-pane-weekday-4.diff

I should say, I considered doing it this way originally, but it causes us to set a timeout every time setDay runs, instead of just once at initialisation.

Comment 10

[[:MakeMyDay]]

Comment on attachment 8997213 [details] [diff] [review]
1479628-today-pane-weekday-4.diff

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

Thanks, that fixes it, r=me

And to state the obvious - this patch needs to be applied on top of version 3 (which already was checked in).

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7574b8f37534
Follow-up: only update weekday deck at end of event loop. r=MakeMyDay DONTBUILD