User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:37.0) Gecko/20100101 Firefox/37.0 Build ID: 20150109190446 Steps to reproduce: Answer to an plain text article or email. Example in newsgroup mozilla.test: <54BB29AA.8060705@hfigge.myfqdn.de> Actual results: The quoting is mangled.

This is caused by the patches of Bug 1113238. If I remove them all is fine again.

SM-Trunk Linux x86_64 is affected also.

Is this fixed by bug 1119503?

(In reply to Takanori MATSUURA from comment #3) > Is this fixed by bug 1119503? It should be retested after that bug is fixed, but according to comment 0, that seems unlikely. :(

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #4) > (In reply to Takanori MATSUURA from comment #3) > > Is this fixed by bug 1119503? > > It should be retested after that bug is fixed, but according to comment 0, > that seems unlikely. :( Err, make that comment 1. Alfred, I assume reverting the 2nd patch on that bug doesn't change anything, and it's only the first one, right? Any chance you can get me the source HTML for the message that you are replying to? Also, are you comfortable with using a debugger such as gdb? If yes, it would be *hugely* helpful if you could set a breakpoint on both nsPlainTextSerializer::IsElementPreformatted and nsXHTMLContentSerializer::IsElementPreformatted and see which one is called and get me some full backtraces. If you need help with doing that, I'd be happy to. Thanks!

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #5) > Alfred, I assume reverting the 2nd patch on that bug doesn't change > anything, and it's only the first one, right? I think Hartmut has already tested that. @Hartmut: It was the It was the second patch - right? > Any chance you can get me the > source HTML for the message that you are replying to? It was also a plain-text message. I can upload the source of it. But, what do you mean with HTML? > would be *hugely* helpful if you could set a breakpoint on both > nsPlainTextSerializer::IsElementPreformatted and > nsXHTMLContentSerializer::IsElementPreformatted and see which one is called > and get me some full backtraces. Ok, I'll do that. You'll get the results tomorrow.

This is a shorter message.

(In reply to Alfred Peters from comment #6) > @Hartmut: It was the It was the second patch - right? Backout of 'Make our...' was not longer possible, so I tried a build with backout of 'Part 2'. The build succeeded, but the problem with the mangling was still there. So the culprit has to be 'Make our...'. Further testing is not possible at the moment due to a bustage on SM-Trunk Linux x86_64.

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #5) > and get me some full backtraces. Here you got it. The buildrevision is: comm-central changeset 17370:46a88d5a7720 mozilla-central changeset 224444:6446c26b45f9

Thanks a lot, Alfred. I have a possible idea for a fix, let me write a patch that you can try out...

Comment on attachment 8554012 [details] [diff] [review] Fall back to looking at the tag for determining if an element is preformatted when there is no style information available Alfred, can you please test this patch? It it doesn't apply cleanly, please just apply the nsPlainTextSerializer.cpp hunk manually. Thanks!

There is a problem applying this patch for SM-Trunk Linux x86_64. hafi@i5_64 ~/hg-moz/src/mozilla $ patch --dry-run -p1 < ~/ztmp/p2/attachment.cgi\?id\=8554012 patching file dom/base/nsPlainTextSerializer.cpp Hunk #1 FAILED at 1792. 1 out of 1 hunk FAILED -- saving rejects to file dom/base/nsPlainTextSerializer.cpp.rej patching file dom/base/test/TestPlainTextSerializer.cpp It may be that there is currently a bustage also for TB, so Alfred may not be able to build.

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #13) > Comment on attachment 8554012 [details] [diff] [review] > Alfred, can you please test this patch? Yes, that looks good. It does the trick. > It it doesn't apply cleanly, please > just apply the nsPlainTextSerializer.cpp hunk manually. Thanks! actually I build without testing.

(In reply to Alfred Peters from comment #15) > Created attachment 8554103 [details] > answer_with_ att8553469.txt Wrong number in the name. "... att8554012.txt" would have been correct.

Comment on attachment 8554012 [details] [diff] [review] Fall back to looking at the tag for determining if an element is preformatted when there is no style information available r=me

(In reply to Alfred Peters from comment #15) > (In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, > needinfo? me!) from comment #13) > > Comment on attachment 8554012 [details] [diff] [review] > Yes, that looks good. It does the trick. Unfortunately, not completely. If I reply only with selected text, then the quotations are still broken.

(In reply to Takanori MATSUURA from comment #3) > Is this fixed by bug 1119503? Just for the records: No, those patches have no effect on this bug.

> and get me some full backtraces. This time: Reply with selected text comm-central changeset 17370:46a88d5a7720 mozilla-central changeset 224444:6446c26b45f9 with attachment 8554012 [details] [diff] [review] implemented!

(In reply to Alfred Peters from comment #18) > If I reply only with selected text, then the quotations are still broken. So they are, when 'copy and paste' out of the edit window. And still another variant: 'copy and paste' out of the message-pane doesn't remove the line-breaks, but instead removes the quoting signs.

Alfred, thanks a lot for testing this. The issues you mentioned in comment 18 onwards are probably separate issues, do you mind please filing separate bugs for them with as much info as you have? The stack trace in comment 20 seems very normal, and it's not examining any edge case as far as I can tell, so please include some information on what it is that you're selecting exactly, etc. Thanks!

(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #22) > tell, so please include some information on what it is that you're selecting > exactly, Doesn't really matter. If I select almost everything from the original message and reply or copy almost everything from the edit window, the result looks exactly like attachment 8553459 [details]. > The issues you mentioned in comment > 18 onwards are probably separate issues, I would bet, they are all coursed by Bug 1113238 > do you mind please filing separate > bugs for them with as much info as you have? If you think so, I'll do that.

(In reply to Alfred Peters from comment #24) > > The issues you mentioned in comment > > 18 onwards are probably separate issues, > > I would bet, they are all coursed by Bug 1113238 Yes, it could be, but the fixes are different than this one (given that this patch didn't fix those issues) so it's better for release tracking purposes to have each symptom on file separately so that we can track which ones are fixed for a given release. If some of those separate issues end up being the same underlying problem, we can dupe the bugs later. > > do you mind please filing separate > > bugs for them with as much info as you have? > > If you think so, I'll do that. Thank you! Please make sure that I'm CCed on the bugs. I will try to fix as many as I can before I go on vacation on Wednesday.

[Tracking Requested - why for this release]: This breaks Thunderbird as noted in comment 0.

(In reply to Alfred Peters from comment #24) > (In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, > > The issues you mentioned in comment > > 18 onwards are probably separate issues, > I would bet, they are all coursed by Bug 1113238 I would have lost that bet. In fact these issues are a bit older.

(In reply to Alfred Peters from comment #28) > (In reply to Alfred Peters from comment #24) > > (In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, > > > The issues you mentioned in comment > > > 18 onwards are probably separate issues, > > I would bet, they are all coursed by Bug 1113238 > > I would have lost that bet. In fact these issues are a bit older. Right. Just tested with a SM-Trunk of last December. There may already be an entry in bugzilla for that. Anyway, it should not be too difficult to find the regression range. Best be done in a new bug or the potential old one, though. ;)

I've alrad(In reply to Hartmut Figge from comment #29) > (In reply to Alfred Peters from comment #28) > > I would have lost that bet. In fact these issues are a bit older. > Right. Just tested with a SM-Trunk of last December. There may already be an > Best be done in a new bug or the potential old one, though. ;) JFTR: Bug 1125956

Comment on attachment 8554012 [details] [diff] [review] Fall back to looking at the tag for determining if an element is preformatted when there is no style information available Approval Request Comment [Feature/regressing bug #]: bug 1119503 [User impact if declined]: missing linebreaks with copy/paste [Describe test coverage new/current, TreeHerder]: new test added here for testing the fallback-to-tag behaviour [Risks and why]: Ehsan also told me to use this as the risk analysis: "The risk on this patch set is moderate. We have tried to not change the existing behavior except for only the well known cases, but this code is really old and not very well understood. I (ehsan) would like these fixes to be backported on Aurora as soon as we can to get broader testing, and will be on the hook for fixing any regressions found." [String/UUID change made/needed]: None

Comment on attachment 8554012 [details] [diff] [review] Fall back to looking at the tag for determining if an element is preformatted when there is no style information available Small fix required for bug 1119503. Aurora+