User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2 Build Identifier: 3.4.1 Upon upgrading to 3.4.1, the text returned for description is different, resulting in false incoming notification in Mylyn (http://eclipse.org/mylyn). This difference is due to line breaks being present in the xml for the description (possibly other fields as well). There were no line breaks present upon original submit (at look at the raw data in the database also reveals no additional line breaks). It appears as though the line wrapping being applied for html presentation is being applied to the xml output as well. Reproducible: Always Steps to Reproduce: 1.Post a long line as a comment to a 3.2 repository 2.Upgrade the repository to 3.4.1 3.Open the task in Mylyn and see false incoming on description due to wrapping in xml Expected Results: Presentation wrapping should not be applied to XML output.

Which fields are affected?

Yes, this is happening because Bugzilla::Bug uses format_comment, which wraps text. format_comment should not wrap the text, I think.

@Frederic: only comments are affected @Max: Thanks for your hint. Based on that I tried this patch and it works fine for us. Comments are not getting wrapped in the XML, but still in the WebUI. Not sure though if that is the best way to do it.

Comment on attachment 393634 [details] [diff] [review] suggested patch No, this is incorrect. You'll want to look at the patch that originally implemented format_comment to see what you have to revert as far as where wrapped_comment goes and where you have to wrap comments. Also, check out our development process so you can be sure that if you submit another patch, it gets reviewed: http://wiki.mozilla.org/Bugzilla:Developers

Thanks for your looking into this Max. Excuse me if I am wrong, but I am totally new to the Bugzilla sources and tried my best to understand what was and is going on in regards to the comment wrapping... As for the patch that implemented the new format_comment.txt.tmpl, it seems to me that the formatting (adding the "duplicate of" and such comments) of comments was done in code (where format_comment is called now), and there was no wrapping done there (at least non that I could find). That's why I thought by removing the wrapping from the template I basically reverted the changes (but leaving the added comments as is) In detail: h3. version from 3.3.4 bc.. # Format language specific comments. This routine must not update # $comment{'body'} itself, see BugMail::prepare_comments(). sub format_comment { my $comment = shift; my $body; if ($comment->{'type'} == CMT_DUPE_OF) { $body = $comment->{'body'} . "\n\n" . get_text('bug_duplicate_of', { dupe_of => $comment->{'extra_data'} }); } elsif ($comment->{'type'} == CMT_HAS_DUPE) { $body = get_text('bug_has_duplicate', { dupe => $comment->{'extra_data'} }); } elsif ($comment->{'type'} == CMT_POPULAR_VOTES) { $body = get_text('bug_confirmed_by_votes'); } elsif ($comment->{'type'} == CMT_MOVED_TO) { $body = $comment->{'body'} . "\n\n" . get_text('bug_moved_to', { login => $comment->{'extra_data'} }); } else { $body = $comment->{'body'}; } return $body; } p. As you can see, there was no wrapping in the format_comment, and I could not find it whereever format_comment is called either.

Hey thomas. That's right, you have to find where wrap_comment was removed and put it back.

Ah, I get it. It seems bug 457657 introduced this new formatting, and was just intended for emails. However, the change also affected XML content. Pretty sure there is a smarter way about this, but with my limited perl knowledge, this is all I can do to fix it. With my previous patch, HTML was wrapped, XML was not (as intended), but also Email was not wrapped (that is what you meant by "find where it was removed

sorry, hit the wrong key...., continuing comment 7: ... With my previous patch, HTML was wrapped, XML was not (as intended), but also Email was not wrapped (that is what you meant by "find where it was removed" I guess). With the v2 patch email now gets wrapped again. I know this is not pretty, but it fixes the regression introduced.

Me again. didn't submit the previous patch correctly, here it goes. Previous 2 comments still apply to this patch.

I don't think Gerv is the right reviewer. Maybe one of the people who wrote or reviewed the original patch would be better.

Comment on attachment 393816 [details] [diff] [review] patch v3 :) I used the linked document to find the reviewer for BugMail.pm. I will try Marc, who provided the original patch.

Comment on attachment 393816 [details] [diff] [review] patch v3 This looks really good to me at first glance. I'll take a closer look as soon as possible.

Hi Marc, Eclipse.org will be moving to 3.4.1 this weekend. Any chance of iterating on the patch this week?

Comment on attachment 393816 [details] [diff] [review] patch v3 I can already tell you that this will break everywhere that we were previously using comment wrapping--you'll want to check the original patch and revert the correct sections.

We're running with this patch here and wrapping still works in the web ui and in emails but the xml output is fixed. Of course there may be other scenarios that I'm unaware of, Marc?

Wrapping is only working in the UI because you are depending on the CSS to do it for you. It is actually going to be much wider than you're used to and will vary between browsers.

Fixing this bug may also fix bug 514703, or vice versa.

(In reply to comment #16) > Wrapping is only working in the UI because you are depending on the CSS to do > it for you. It is actually going to be much wider than you're used to and will > vary between browsers. I don't think this is actually a problem. Bug 457657 is the bug where the wrapping was introduced to format_comment. (attachment 379597 [details] [diff] [review] is very close to what was committed to CVS.) The only place where wrapping was removed as in BugMail.pm - which Thomas's patch reintroduces. Wrapping is still done for the web UI in ./template/en/default/bug/comments.html.tmpl The handling for email will be slightly different after this patch - because any text introduced in a long-winded translation of the format_comment template will be also be wrapped. But if anything, that seems like a change for the better. (Don't know if spontaneous reviews from random non-developers are appreciated, but the patch looks correct to me; I don't see any problems in it.)

Comment on attachment 393816 [details] [diff] [review] patch v3 I like patch v3. I like the idea of handling comments generally unwrapped, and do the wrapping only when it needs to happen. And as you say, show_bug.cgi and bugmail are already covered. Unfortunately, the patch doesn't apply to HEAD any more, so I can't test; please upload an unrotted patch. I'll see whether I come accross other places in need of wrapping.

Hi Marc Thanks for looking at this. Here is an updated version that works against HEAD again.

As we got no comment from Marc for almost a month, I'm going to review this patch.

Comment on attachment 403812 [details] [diff] [review] patch v.4 >Index: webtools/bugzilla/Bugzilla/BugMail.pm >+ $comment->{body} = ($comment->{'already_wrapped'} ? $comment->{body} : wrap_comment($comment->{body})); Nit: parentheses are useless and can be removed on checkin (I know they were present in the original code; that's not a big deal). The patch is correct and is working fine, for bugmails, XML files and bugs displayed in the web browser (including format for printing, aka long format). r=LpSolit And yes, the wrapping happens independently of CSS; comments.html.tmpl handles this correctly.

tip: Checking in Bugzilla/BugMail.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v <-- BugMail.pm new revision: 1.129; previous revision: 1.128 done Checking in template/en/default/bug/format_comment.txt.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/format_comment.txt.tmpl,v <-- format_comment.txt.tmpl new revision: 1.3; previous revision: 1.2 done 3.4.2: Checking in Bugzilla/BugMail.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v <-- BugMail.pm new revision: 1.124.2.5; previous revision: 1.124.2.4 done Checking in template/en/default/bug/format_comment.txt.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/format_comment.txt.tmpl,v <-- format_comment.txt.tmpl new revision: 1.1.2.3; previous revision: 1.1.2.2 done

Thanks Frederic for reviewing this and applying the fix!

Thanks for picking this up, Frédéric.

No problem. Why resetting the version field??

(In reply to comment #26) > No problem. Why resetting the version field?? I don't know how this happened. It was not intentional.