Remove useless newlines in replies sent by email_in.pl

RESOLVED FIXED in Bugzilla 3.0

Status

()

Bugzilla
Incoming Email
--
minor
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Frédéric Buclin, Assigned: Frédéric Buclin)

Tracking

3.0.3
Bugzilla 3.0
Bug Flags:
approval +
blocking3.2 -
approval3.0 +

Details

Attachments

(1 attachment, 1 obsolete attachment)

875 bytes, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

10 years ago
When you pass illegal changes to email_in.pl, it replies to the sender about what's wrong:

You tried to change the 
    Priority field 
      from P4
      to P1
    , but only
      the assignee or reporter 
      of the bug, or
    a sufficiently empowered user may change that field.


Frédéric Buclin wrote:
> > @priority = P1


As you can see, the message is pretty ugly, because every newline from user-error.html.tmpl is displayed as is in the reply. These newlines are not a problem from the web browser because HTML doesn't render them. But they are obviously a problem in a text message. We should remove newlines from error messages before sending the email back to the sender.
Flags: blocking3.2?

Comment 1

10 years ago
  This could be fixed by wrapping it in "FILTER collapse". However, if there was any <pre> formatting in the message, it would be destroyed. I think that's OK, though.

  This isn't significant enough for me to actually block a release on it, but I agree that it'd be nice to fix.
Severity: normal → minor
Flags: blocking3.2? → blocking3.2-
(Assignee)

Comment 2

10 years ago
Created attachment 318862 [details] [diff] [review]
patch, v1
Assignee: incoming.email → LpSolit
Status: NEW → ASSIGNED
Attachment #318862 - Flags: review?(mkanat)

Comment 3

10 years ago
Comment on attachment 318862 [details] [diff] [review]
patch, v1

  No, you should just wrap message.txt.tmpl with FILTER collapse, I think.
Attachment #318862 - Flags: review?(mkanat) → review-

Comment 4

10 years ago
Comment on attachment 318862 [details] [diff] [review]
patch, v1

  Okay, since you pointed out on IRC that ThrowUserError doesn't use message.txt.tmpl (nor would CodeError or TemplateError, etc.) this is the best and most centralized way to do this.

However, this should be s/[\s\n]+/ /gms I think, no?
(Assignee)

Comment 5

10 years ago
Comment on attachment 318862 [details] [diff] [review]
patch, v1

message.txt.tmpl is not called by ThrowUserError().
Attachment #318862 - Flags: review- → review?(mkanat)

Comment 6

10 years ago
Comment on attachment 318862 [details] [diff] [review]
patch, v1

  See above comment about regex.
Attachment #318862 - Flags: review?(mkanat) → review-
(Assignee)

Comment 7

10 years ago
Created attachment 318864 [details] [diff] [review]
patch, v2

The result is pretty much the same. :)
Attachment #318862 - Attachment is obsolete: true
Attachment #318864 - Flags: review?(mkanat)

Comment 8

10 years ago
Comment on attachment 318864 [details] [diff] [review]
patch, v2

Looks good to me.
Attachment #318864 - Flags: review?(mkanat) → review+
(Assignee)

Updated

10 years ago
Flags: approval3.0+
Flags: approval+
(Assignee)

Comment 9

10 years ago
tip:

Checking in email_in.pl;
/cvsroot/mozilla/webtools/bugzilla/email_in.pl,v  <--  email_in.pl
new revision: 1.19; previous revision: 1.18
done

3.0.4:

Checking in email_in.pl;
/cvsroot/mozilla/webtools/bugzilla/email_in.pl,v  <--  email_in.pl
new revision: 1.5.2.12; previous revision: 1.5.2.11
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.