Bugmail uses the timezone of the changee instead of the user receiving the mail

RESOLVED FIXED in Bugzilla 3.4

Status

()

Bugzilla
Email Notifications
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: reed, Assigned: reed)

Tracking

3.4.4
Bugzilla 3.4
Bug Flags:
approval +
approval3.4 +

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
Created attachment 417436 [details] [diff] [review]
patch - v1

Instead of using the user's chosen timezone for displaying the comment header in bugmail, it seems the changee's timezone is used.

Untested patch attached.
Attachment #417436 - Flags: review?(mkanat)

Comment 1

8 years ago
Comment on attachment 417436 [details] [diff] [review]
patch - v1

This looks basically right. However:

* "undef" doesn't exist in TT, that's just another variable name. It will just pass an empty string, so the thing that needs to be done there is just to pass an empty string and then modify format_time to check $format instead of just "defined $format".

* I think we should just pass the whole $user object as to_user, to the template. I've wanted that many times anyhow.
Attachment #417436 - Flags: review?(mkanat) → review-
(Assignee)

Comment 2

8 years ago
(In reply to comment #1) 
> * "undef" doesn't exist in TT, that's just another variable name. It will just
> pass an empty string

I wondered about that, but a mxr search for "undef" under template/ returned a few things, so I thought it might work.

http://mxr.mozilla.org/bugzilla/search?string=undef&find=%2Ftemplate&findi=&filter=^[^\0]*%24&hitlimit=&tree=bugzilla

> * I think we should just pass the whole $user object as to_user, to the
> template. I've wanted that many times anyhow.

Ok, can do. New patch coming up in a bit.
(Assignee)

Comment 3

8 years ago
Created attachment 417607 [details] [diff] [review]
patch - v2 (tip)

How about this?
Attachment #417436 - Attachment is obsolete: true
Attachment #417607 - Flags: review?(mkanat)

Comment 4

8 years ago
Comment on attachment 417607 [details] [diff] [review]
patch - v2 (tip)

That looks pretty reasonable. I'll test it before checkin.

For the 3.4 branch, we shouldn't take away "to", because that could break somebody's custom email template.
Attachment #417607 - Flags: review?(mkanat) → review+

Comment 5

8 years ago
(Another patch for 3.4 that doesn't remove "to" would help out with the checkin process.)
Flags: approval3.4?
Flags: approval+
(Assignee)

Comment 6

8 years ago
Created attachment 417608 [details] [diff] [review]
patch - v1 (3.4 branch)
Attachment #417608 - Flags: review?(mkanat)

Updated

8 years ago
Attachment #417608 - Flags: review?(mkanat) → review+

Updated

8 years ago
Flags: approval3.4? → approval3.4+
(Assignee)

Updated

8 years ago
Attachment #417607 - Attachment description: patch - v2 → patch - v2 (tip)
(Assignee)

Comment 7

8 years ago
tip:

Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.132; previous revision: 1.131
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.100; previous revision: 1.99
done
Checking in template/en/default/email/newchangedmail.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/email/newchangedmail.txt.tmpl,v  <--  newchangedmail.txt.tmpl
new revision: 1.18; previous revision: 1.17
done

BUGZILLA-3_4-BRANCH:

Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.124.2.6; previous revision: 1.124.2.5
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.86.2.5; previous revision: 1.86.2.4
done
Checking in template/en/default/email/newchangedmail.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/email/newchangedmail.txt.tmpl,v  <--  newchangedmail.txt.tmpl
new revision: 1.12.2.5; previous revision: 1.12.2.4
done
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.