Closed Bug 311563 Opened 20 years ago Closed 17 years ago

Make whining permit utf8 in whining reports

Categories

(Bugzilla :: Whining, defect)

Product:
Bugzilla
Component:
Whining
Version:
2.21
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 3.2

People

(Reporter: bugreport, Assigned: Wurblzap)

References

Details

Attachments

(3 files, 1 obsolete file)

Testing script
1.35 KB, text/plain
Details
Patch 1.0.1
2.67 KB, patch
mkanat
: review+
Details | Diff | Splinter Review
screenshot
65.99 KB, image/png
Details
If a bug requires utf8 and utf8 is enabled, whining should handle it properly.
Assignee: erik → whining
I suspect bug 353711 will fix this.
Depends on: 353711
Whiteboard: [blocker will fix]
Bug 353711 didn't cut it. whine.pl does stuff like this: push @{$args->{'alternatives'}}, { 'content' => $msg, 'type' => 'text/html', }; A solution might be to specify 'text/html; charset=UTF-8' in the utf8 case (and the same for the text/plain part of the message).
Whiteboard: [blocker will fix]
Target Milestone: --- → Bugzilla 3.2
Attached patch Patch (obsolete) — — Splinter Review
Scrap the proposal of comment 2. The patch introduces a new bug until http://rt.cpan.org/Public/Bug/Display.html?id=24858 is fixed: while it correctly handles encoding, all parts of multipart messages are announced and sent as being text/plain, overriding whatever content type they really are.
Assignee: whining → wurblzap
Status: NEW → ASSIGNED
Would just like to say that I think this is a very important issue - Whines are totally unusable for anyone working with Asian languages.
Ricardo, CCing you here after seeing your input at bug 374424... Do you have a suggestion on how to proceed here?
I need more context. Link to the whine.pl? (I'm happy to have a look, but not very interested in digging through the source to find things.) As I may have said before, the correct solution to MANY of Bugzilla's woes are probably found in using Mail::Message, part of Mail-Box distro. It's larger than Email::MIME, but much, much, much more robust for doing things with i18n and other wonky bits.
Ricardo, thanks for your reply. This is about the second part about the Email-MIME RT mentioned in comment 3 here. I set up a testing script which shows the bug I'm facing. The bug is that header lines of mail parts aren't recognized correctly. I get the output Content type: **undefined** Content type: **undefined** instead of the expected Content type: text/plain Content type: text/html (modulo charset information; I don't know whether to expect charset information) as output of the following (non-Bugzilla-dependent) script -- maybe you can find out whether there's really something wrong with Email-MIME? #!/usr/bin/perl -w use strict; use Email::MIME::Modifier; my $plain_mail = <<EOT From: bugzilla-daemon\@some.where To: receiver\@some.where Subject: [bz-head] Doubles MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----------6052--1183018980-----" Date: Thu, 28 Jun 2007 10:23:00 +0200 ------------6052--1183018980----- Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Double B=C3=BCgs ------------6052--1183018980----- Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head><title>[bz-head] Doubles</title></head> <body><p align=3D"left">Double B=C3=BCgs</p></body> </html> ------------6052--1183018980------- EOT ; my $mail = new Email::MIME($plain_mail); $mail->walk_parts(sub { my ($part) = @_; return if $part->parts > 1; # Top-level my $content_type = $part->content_type || '**undefined**'; print "Content type: $content_type\n"; });
Thanks for the details. I will have a look at this, this weekend. (Possibly soon. It depends on the success of the Baby Naptime project.)
Flags: blocking3.0.3?
Flags: blocking3.0.3? → blocking3.1.3?
Flags: blocking3.1.3? → blocking3.1.3+
Confirmed independently with Bugzilla-ru: https://bugzilla.mozilla-russia.org/show_bug.cgi?id=349 Suggested template fix is no more than workaround: https://bugzilla.mozilla-russia.org/attachment.cgi?id=114
Marc, is this bug still valid with the work done on UTF-8 recently?
I very much think so, yes. With http://rt.cpan.org/Public/Bug/Display.html?id=24858 unfixed, we're not able to mark parts of multipart messages to be UTF-8.
I think that bug got under my radar somehow. I'll try to have a look at it soon. In the meantime, patches welcome.
Flags: blocking3.2+
Flags: blocking3.1.3-
Flags: blocking3.1.3+
Do we use multipart-mime.txt.tmpl to generate anything beyond text/plain and text/html? If we don't -- why not putting it as ... Content-type: [% part.type +%]; charset=utf-8 ... ?
(In reply to comment #13) > If we don't -- why not putting it as > Content-type: [% part.type +%]; charset=utf-8 For the reason Marc gave above, charset="UTF8" is dropped (if I understood correctly). Ricardo, we really need to fix this bug. Can you help, please? Email::* also throws "Wide character in print at Bugzilla/Mailer.pm line 137" when using strings as given in bug 423439. Seems due to $email->as_string.
As this is a blocker, we may need to work around this outside of Email::MIME as long as Ricardo is missing.
Comment #11 points to http://rt.cpan.org/Public/Bug/Display.html?id=24858 That RT bug discusses the combination of Email::MIME-1.857 and Email::Simple-1.998. Email::MIME 1.857 was replaced over a year ago. Email::Simple 1.998 was also. The tests that the bug (in rt.cpan.org) says failed with that combination pass with the current versions. Can you please indicate (here, if you prefer, but preferably in the bug in rt.cpan.org) what the *current* problem is? If this is still a problem, a failing test on the current versions would go a long way to getting this problem easy for me to address. Sorry for my mysterious absence. I've been quite swamped at work, and dealing with the Email:: modules has not been getting much active attention. I needed (a) a vacation and (b) to be pestered. Please let me know specifically what is not working with the current versions. Please, also, understand that fixes for year-old versions are unlikely to occur.
From memory, I managed to reproduce the bug using Email::MIME 1.860 and Email::Simple 2.003. The problem is described in details in comment 7: Content-Type: text/plain; charset="UTF-8" doesn't work and the whole line is ignored, i.e. both the charset and the content type, which doesn't let us add utf8 to emails.
Attached file Testing script —
Whoa yes, the attached testing script succeeds now with my Email-MIME 1.861. I kinda expected this to happen only after I saw some activity over at the RT bug. I'll check whether this means that we're settled now.
Attachment #314277 - Attachment mime type: application/octet-stream → text/plain
Attached patch Patch 1.0.1 — — Splinter Review
It seems we're clear now :) This is the original patch, unrotted. Works well with mail_delivery_method set to “Test” (I can't test other settings).
Attachment #270170 - Attachment is obsolete: true
Attachment #314312 - Flags: review?(mkanat)
Note: please especially verify the package requirement I added. It may be it's Email::Simple we need a certain version of, not Email::MIME.
Comment on attachment 314312 [details] [diff] [review] Patch 1.0.1 This *looks* correct. As long as we don't ever have nested multipart messages (and we don't now and have no plans to), we're good. I trust you tested this with normal mail, flag mail, and whine mail, yeah?
Attachment #314312 - Flags: review?(mkanat) → review+
I think this should at least be tested with real emailing (Sendmail, SMTP) before approval.
Flags: approval?
(In reply to comment #22) > I think this should at least be tested with real emailing (Sendmail, SMTP) > before approval. I will apply the patch on landfill as soon as landfill is upgraded, so that Marc and others can test it. I will paste the URL here when it's ready.
(In reply to comment #21) > (From update of attachment 314312 [details] [diff] [review]) > This *looks* correct. As long as we don't ever have nested multipart messages > (and we don't now and have no plans to), we're good. True, but the original code wasn't recursive either. It's simple to fix this as soon as it's needed. > I trust you tested this with normal mail, flag mail, and whine mail, yeah? Yes. But as said in comment 19, with testfile only. I'd feel better if this could get tested with live mail before check-in. (In reply to comment #23) > I will apply the patch on landfill as soon as landfill is upgraded, so that If you don't mind, I'd rather do it myself so that I can easily tweak things there if needed.
Ok, tested all three kinds of mail successfully live at https://landfill.bugzilla.org/bz311563/.
Attached image screenshot —
Here is a screenshot of emails I get without (top) and with the patch applied (bottom). So it seems that the patch also works nicely with Email::MIME 1.860. Could we decrease the min requirement for it, please? This would at least make Mandriva users happy as Email::MIME 1.860 (together with Email::Simple 2.003) is what is released in the latest Mandriva.
http://search.cpan.org/src/RJBS/Email-MIME-1.861/Changes tells me Email::MIME 1.861 requires Email::Simple 2.003, but Email::MIME 1.860 does not. I don't know which upgrade fixed the bug -- Ricardo, can you shed some light on this? What minimum versions do we require here? If we don't know for sure, we should play safe and require Email::Simple 2.003. This needs to be an explicit requirement unless we require Email::MIME 1.861 or higher. Requiring Email::Simple 2.003 doesn't hurt people, I'd say -- if they can get Email::MIME 1.860, then they can get Email::Simple 2.003, too. I'll upload a patch doing so unless Ricardo tells us otherwise.
Hrm... ok, go with Email::MIME 1.861, so that we are sure to not have a broken Email::Simple or something. No need to mess checksetup.pl with a too long list of module checks.
Flags: approval? → approval+
All right. Checking in Bugzilla/Mailer.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Mailer.pm,v <-- Mailer.pm new revision: 1.20; previous revision: 1.19 done Checking in Bugzilla/Install/Requirements.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v <-- Requirements.pm new revision: 1.46; previous revision: 1.45 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: