When creating plain text news articles or emails with flowing text paragraphs, the text is not wrapped

RESOLVED FIXED in Firefox 37

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: infofrommozilla, Assigned: Ehsan)

Tracking

({regression})

Trunk
mozilla39
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox36 unaffected, firefox37+ fixed, firefox38+ fixed, firefox39+ fixed, firefox-esr31 unaffected)

Details

Attachments

(4 attachments)

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:

Create a new e-mail or news article and write a long (> ~100 chars) paragraph with no manual line brake.


Actual results:

Although the text wraps in the editor that paragraph will be sent without a line break.
Blocks: 1113238
Component: Untriaged → Message Compose Window
Keywords: regression
This is caused by the patches of Bug 1113238. If I remove them all is fine again.
Status: UNCONFIRMED → NEW
Component: Message Compose Window → Composition
Ever confirmed: true
OS: Windows 7 → All
Product: Thunderbird → MailNews Core
[Tracking Requested - why for this release]:

Can you please attach the HTML source of the email?

Also, can you please see which patch in that bug break this?  (I assume that the breakage exists on a build based on mozilla-inbound tip right now?)
Posted file Sent article
Hmm, these don't tell me much about what may be going on... Do you mind setting a breakpoint on nsPlainTextSerializer::IsElementPreformatted and capturing a stack trace similar to what you did in bug 1123062, please?

(Note that I will probably not get to debug and fix this before I return from vacation...)
Component: Composition → Serializers
Flags: needinfo?(infofrommozilla)
Product: MailNews Core → Core
Flags: needinfo?(ehsan)
(In reply to [Away: 1/29-2/20] from comment #2)

>  (I assume that
> the breakage exists on a build based on mozilla-inbound tip right now?)

Does exist a source of the compiled binaries?
If so, where can I find them?
Flags: needinfo?(infofrommozilla)
(In reply to Alfred Peters from comment #6)
> (In reply to [Away: 1/29-2/20] from comment #2)
> 
> >  (I assume that
> > the breakage exists on a build based on mozilla-inbound tip right now?)
> 
> Does exist a source of the compiled binaries?
> If so, where can I find them?

The stuff I was talking about were merged to mozilla-central, so just update your mozilla-central please.
(In reply to [Away: 1/29-2/20] from comment #7)
> > (In reply to [Away: 1/29-2/20] from comment #2)
> > >  (I assume that
> > > the breakage exists on a build based on mozilla-inbound tip right now?)

> The stuff I was talking about were merged to mozilla-central, so just update
> your mozilla-central please.

Yes, the Bug still exists in current Trunk:
   comm-central:  17420:1f549a4e9351
mozilla-central: 226359:bfa194d93aed
(In reply to [Away: 1/29-2/20] from comment #5)
> Hmm, these don't tell me much about what may be going on... Do you mind
> setting a breakpoint on nsPlainTextSerializer::IsElementPreformatted and
> capturing a stack trace similar to what you did in bug 1123062, please?

Done - HTH
(In reply to Alfred Peters from comment #1)
> This is caused by the patches of Bug 1113238.

(In reply to [Away: 1/29-2/20] from comment #2)
> Also, can you please see which patch in that bug break this?

It is Part 1 <https://hg.mozilla.org/mozilla-central/rev/183898a49db1>
Alfred, can you please see if the patch I attached to bug 1125956 also fixes this?  Thanks!
Flags: needinfo?(ehsan) → needinfo?(infofrommozilla)
See Also: → 1125956
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #11)
> Alfred, can you please see if the patch I attached to bug 1125956 also fixes
> this?  Thanks!

No, doesn't work.

The patch changes nsHTMLCopyEncoder::SetSelection().
But the function is not called when sending.
Flags: needinfo?(infofrommozilla)
[Tracking Requested - why for this release]: Bug 1113238 was backported to 37 as well, so this regression is present on that branch too.
Tracking this regression.

Ehasn - Can you take this bug?
Yes, I'm already testing a fix on the try server, should have something up for review later today if this round of the fix turns out green!
Assignee: nobody → ehsan
Flags: needinfo?(ehsan)
This ensures that the plaintext serializer doesn't use the preformatted
text code path if we have encountered a pre-wrap element that
Thunderbird uses (which means setting white-space: pre-wrap and width:
NNch on the body element.)

It also ensures that we use 0 as the wrap column number passed down to
the plaintext serializer, instead of -1, which this code seems to be
unable to handle properly.
Attachment #8571570 - Flags: review?(bzbarsky)
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #17)
> Created attachment 8571570 [details] [diff] [review]

Works for my SM-Trunk Linux x86_64.
Comment on attachment 8571570 [details] [diff] [review]
Fix serialization of the pre-wrap elements that Thunderbird relies on

Do you mind adding some docs for mPreFormatted describing what that member means?  Because it doesn't mean what I assumed based on the name....

Of course maybe it needs a different name.

r=me
Attachment #8571570 - Flags: review?(bzbarsky) → review+
(In reply to Not doing reviews right now from comment #19)
> Comment on attachment 8571570 [details] [diff] [review]
> Fix serialization of the pre-wrap elements that Thunderbird relies on
> 
> Do you mind adding some docs for mPreFormatted describing what that member
> means?  Because it doesn't mean what I assumed based on the name....
> 
> Of course maybe it needs a different name.

I renamed it to mPreFormattedMail:

https://hg.mozilla.org/integration/mozilla-inbound/rev/87884288be1e
https://hg.mozilla.org/integration/mozilla-inbound/rev/caa3bfedd7b8
Comment on attachment 8571570 [details] [diff] [review]
Fix serialization of the pre-wrap elements that Thunderbird relies on

Approval Request Comment
[Feature/regressing bug #]: Bug 1113238.
[User impact if declined]: See comment 0.
[Describe test coverage new/current, TreeHerder]: Tested locally and on the try server.
[Risks and why]: Low risk, this patch fixes a mistake in the patch for bug 1113238, the logic after the fix is clear.  Note that this only affects Thunderbird, but it's possible that this code also runs on some unfortunate website out there matches the DOM structure that Thunderbird relies on here.  But for risk analysis this is good news, since it means that in all likelyhood, this patch and the original bug do not affect any websites, which makes the bug very Thunderbird specific.
[String/UUID change made/needed]: None.
Attachment #8571570 - Flags: approval-mozilla-beta?
Attachment #8571570 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/87884288be1e
https://hg.mozilla.org/mozilla-central/rev/caa3bfedd7b8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8571570 [details] [diff] [review]
Fix serialization of the pre-wrap elements that Thunderbird relies on

Given that this affect of this fix is very likely limited to Thunderbird and that this is a correctness fix from a prior patch, let's take this in Beta 3 even though this only recently landed on m-c. Beta+ Aurora+
Attachment #8571570 - Flags: approval-mozilla-beta?
Attachment #8571570 - Flags: approval-mozilla-beta+
Attachment #8571570 - Flags: approval-mozilla-aurora?
Attachment #8571570 - Flags: approval-mozilla-aurora+
The patch landed with a test case.  No other tests are necessary.
Flags: in-testsuite? → in-testsuite+
Missed the gtest there, sorry.
You need to log in before you can comment on or make changes to this bug.