1298390 - Wrong timezeone on month/week print view

Status: RESOLVED FIXED

(Calendar :: Printing, defect)

Description

[louis]

Attachment: mensualview.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36

Steps to reproduce:

Add an external carldav calendar.
Create an event on this calendar.







Actual results:

Mensual/Weekly Grid view does not display events in the proper timezone
This bug does not happen for local calendars neither in the print as list view.
My timezone is Europe/Paris

I think this is not the same problem with https://bugzilla.mozilla.org/show_bug.cgi?id=1151011&GoAheadAndLogIn=1

Comment 1

[[:MakeMyDay]]

Are there any messages in the error log when reproducing the issue? Can you please and example ics for which this is an issue? I assume you have configured your default timezone in the preferences?

Comment 2

[louis]

My thunderbird and lightning timezone are both set to Europe/Paris.
There isn't any bug shown in the console.
What should I send to you ? This is a carldav server i use.

I can add that this bug doesn't occur on recurrent events, as the second image i upload.

Comment 3

[louis]

Attachment: recurrent.png

Comment 4

[louis]

I've found the bug ! In calPrintUtils.js in getItemIntervalString() (l61) the timezoned value (start & end) are used only for day comparison.
Not for time displaying - line 182 to 192 (startDate & endDate original values are used).

Comment 5

[louis]

Attachment: printcalendartimezone.patch

Comment 6

[[:MakeMyDay]]

Comment on attachment 8785474 [details] [diff] [review]
printcalendartimezone.patch

Louis, thanks for digging into this and submitting your first patch!

I assume you created the diff in the wrong order, as we don't have startTimezoned and endTimezoned in our code base. Apart from that, you don't need that new variables at all, you could simply use start and end to feed into formatTime here, as these are already clones converted to the local timezone.

Unfortunately, you cannot use patches made on the packaged version of Lightning to check them in directly, because file location differ from that compared to the repository.

To get to an applicable patch, you basically would need to install Hg, checkout the source code (which may take some time, because the tree is quite hugh), create a proper patch, submit it and ask for review. See [1] for how to do this. (for now, you can stop reading after the "How can I generate a patch for somebody else to check-in for me?" section).

I hope this obstackle will not prevent you from submitting an updated patch, new contributors are always welcome! And once you've done the initial setup, you would have everything in shape for future patches ;-)

[1] https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial

Comment 7

[louis]

Hello,

I used new variables because of 
start.isDate = true;
end.isDate = true;
Theses variables makes date all day no ? And that would impact the formatTimeInterval function below ?
For optimization i can directly start and end before isDay assignment. What do you think of that ? 

I will generate the proper patch very soon !

Comment 8

[[:MakeMyDay]]

Yes, you're right. It looks like you can safely do the timezone conversion already when assigning startDate/endDate. In that case, you also can get rid of the later timezone conversion when assigning start/end.

Thanks for taking care!

Comment 9

[louis]

Attachment: Bug#1298390 - timezone on weekly monthly print view .diff

Comment 10

[[:MakeMyDay]]

Comment on attachment 8786081 [details] [diff] [review]
Bug#1298390 - timezone on weekly monthly print view .diff

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

Louis, thanks for preparing the patch!

The approval requests are not what you want at this stage of the patch submission process. The first thing is to ask for review from an appropriate person by setting the '?' in the review field like you did for the approvals (this applies also, if you upload and updated version of a patch within a review process). If you don't know whom to ask directly, let bugzilla make one or more  suggestion(s) based on the code you're touching by clicking on the suggested reviewers link. Make your pick - maybe decide based one the size of the person's review queue or on previous reviews you've gone through.

For now, I'll take care.

Apart from the whitespace issue below, please modify your commit message to something more brief but meaningful like "Bug 1298390 - Print events in local time in weekly and mothly grid". Also, please append ";r=reviewer" to the commit message, where reviewer should be replaced by the nick or name of the reviewer, you've picked (which means r=MakeMyDay in this case). That is required for checkin latest, but it makes things way easier to add that already when uploading the patch, so that others who will be landing your patch once reviewed don't need to modify the message.

That said, can you please upload an updated patch considering the comments above?

::: calendar/base/modules/calPrintUtils.jsm
@@ +173,5 @@
>  
>          let dateFormatter = cal.getDateFormatter();
>          let defaultTimezone = cal.calendarDefaultTimezone();
> +        startDate = startDate.getInTimezone(defaultTimezone);
> +        endDate = endDate.getInTimezone(defaultTimezone);		

Please remove the whitespaces at the end of this line.

Comment 11

[louis]

Attachment: Bug 1298390 - Print events in local time in weekly and monthly grid.diff

Comment 12

[[:MakeMyDay]]

Comment on attachment 8786098 [details] [diff] [review]
Bug 1298390 - Print events in local time in weekly and monthly grid.diff

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

Looks good now, thank you for the patch!

Comment 13

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/ee588b495c34de32a1f480c37efa80d2314ee5d5
Bug 1298390 - Print events in local time in weekly and monthly grid. r=MakeMyDay