If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Wrong timezeone on month/week print view

RESOLVED FIXED in 5.3

Status

Calendar
Printing
RESOLVED FIXED
a year ago
8 months ago

People

(Reporter: louis, Assigned: louis)

Tracking

Lightning 4.7

Details

(Whiteboard: [timezone])

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

a year ago
Created attachment 8785306 [details]
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

a year ago
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?
Flags: needinfo?(louis)
Whiteboard: [timezone]
(Assignee)

Comment 2

a year ago
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.
(Assignee)

Comment 3

a year ago
Created attachment 8785364 [details]
recurrent.png
(Assignee)

Comment 4

a year ago
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).
(Assignee)

Comment 5

a year ago
Created attachment 8785474 [details] [diff] [review]
printcalendartimezone.patch

Comment 6

a year ago
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
Flags: needinfo?(louis)
Attachment #8785474 - Flags: feedback+

Updated

a year ago
Assignee: nobody → louis
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 7

a year ago
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

a year ago
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!
(Assignee)

Comment 9

a year ago
Created attachment 8786081 [details] [diff] [review]
Bug#1298390 - timezone on weekly monthly print view .diff
Attachment #8785306 - Attachment is obsolete: true
Attachment #8785364 - Attachment is obsolete: true
Attachment #8785474 - Attachment is obsolete: true
Attachment #8786081 - Flags: approval-calendar-beta?(philipp)
Attachment #8786081 - Flags: approval-calendar-aurora?(philipp)

Comment 10

a year ago
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.
Attachment #8786081 - Flags: review-
Attachment #8786081 - Flags: approval-calendar-beta?(philipp)
Attachment #8786081 - Flags: approval-calendar-aurora?(philipp)
(Assignee)

Comment 11

a year ago
Created attachment 8786098 [details] [diff] [review]
Bug 1298390 - Print events in local time in weekly and monthly grid.diff
Attachment #8786081 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Attachment #8786098 - Flags: review?(makemyday)

Comment 12

a year ago
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!
Attachment #8786098 - Flags: review?(makemyday) → review+

Updated

a year ago
Keywords: checkin-needed

Comment 13

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

Updated

a year ago
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 5.3

Updated

a year ago
Duplicate of this bug: 1300857

Updated

9 months ago
Duplicate of this bug: 1329673

Updated

8 months ago
Duplicate of this bug: 1330258
You need to log in before you can comment on or make changes to this bug.