Closed Bug 1384361 Opened 7 years ago Closed 7 years ago

Text alignment gets discarded from HTML compositions when sending (Delivery Format: Auto-Detect; recipients prefer: HTML(!) & Unknown; Unknown)

Categories

(MailNews Core :: Composition, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 56.0

People

(Reporter: thomas8, Assigned: thomas8)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [ux-wysiwyg])

User Story

With TB default settings, any explicit text alignment created with Editor's formatting bar (or Insert > HTML: align="...") in an otherwise simple HTML message is unexpectedly discarded before sending.

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1202227 +++ STR: 1) Compose HTML message to recipient who "prefers to receive messsages formatted as: Unknown (default)". Set "when one or more recipients can't have HTML" to anything which is NOT "just send plaintext" (default: Plaintext AND HTML), to indicate your preference for HTML delivery format. 2) Format some text using the text alignment button from composition toolbar (do not use any other formatting): --- Left Right Center Justified asdf asdfa sdfa asdfasdf asdfasfd uasdf --- Real-life example: Someplace, 6th Sept. 2015 Dear customer, we'd like to invite you to our Fund Raising Dinner All proceeds will go to charity. We hope to welcome you to this extraordinary event. --- 3) Send (to outbox for testing purposes, Ctrl+Shift+Enter) 4) Look at sent message Actual result * All explicit text alignment has been silently dumped by Auto-Detect aka Auto-Degrade. --- Left Right Center Justified asdf asdfa sdfa asdfasdf asdfasfd uasdf --- Someplace, 6th Sept. 2015 Dear customer, we'd like to invite you to our Fund Raising Dinner All proceeds will go to charity. We hope to welcome you to this extraordinary event. --- Expected result: * We must never dump explicit formatting after the fact, with default HTML settings - ux-wysiwyg - ux-control - ux-error-prevention - ux-consistency
Let's fix this.
Attachment #8890141 - Flags: ui-review?(richard.marti)
Attachment #8890141 - Flags: review?(acelists)
Comment on attachment 8890141 [details] [diff] [review] Patch V.1: Tags with align attribute should not be convertible Makes sense.
Attachment #8890141 - Flags: ui-review?(richard.marti) → ui-review+
Comment on attachment 8890141 [details] [diff] [review] Patch V.1: Tags with align attribute should not be convertible Review of attachment 8890141 [details] [diff] [review]: ----------------------------------------------------------------- OK. I think hunting down what is NOT convertible is an endless task. We should have a fixed set of tags that are convertible and everything else isn't. That would be for another rewrite of the function.
Attachment #8890141 - Flags: review?(acelists) → review+
Keywords: checkin-needed
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/8bbe89ca7478 HTML message with align attributes must not be convertible to plaintext. r=aceman
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 56.0
Thanks everyone! Great teamwork, great turnaround time! :) We've really gone a long way to contain the HTML-eating behaviour of Auto-Downgrade.
(In reply to :aceman from comment #3) > I think hunting down what is NOT convertible is an endless task. Yes. And when we're truly done hunting, we may find that the so-called "lossless conversion" is a myth. Even converting <p>'s into Paragraph1Text\n\nParagraph2Text changes the amount of spacing, and plaintext is much more likely to be rendered as fixed-width but when I composed it was variable width, both of which violates ux-wysiwyg. I'm still largely failing to see the benefit of Auto-Downgrade. When there's almost no HTML, the change in message size will be totally insignificant. So what's the real benefit? And is this really a good default for the majority of our users? > We should > have a fixed set of tags that are convertible and everything else isn't. > That would be for another rewrite of the function. That could be tried, but we must ensure best possible performance. We must continue to bail out as soon as we find just *one* tag which isn't convertible. For determining convertibility of each tag, we'll have to analyse which is more performant, checking for non-convertible attributes etc. (and again bail out from the tag-check as soon as we find one), or checking against a list of convertibles. In general, it seems to me that this code has a high performance cost for little gain.
Blocks: 1385650
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #7) > > We should > > have a fixed set of tags that are convertible and everything else isn't. > > That would be for another rewrite of the function. > > That could be tried, but we must ensure best possible performance. We must > continue to bail out as soon as we find just *one* tag which isn't > convertible. For determining convertibility of each tag, we'll have to > analyse which is more performant, checking for non-convertible attributes > etc. (and again bail out from the tag-check as soon as we find one), or > checking against a list of convertibles. In general, it seems to me that > this code has a high performance cost for little gain. Yes I think currently we check all elements and find the highest non-convertible value (nsMsgCompose::_NodeTreeConvertible). We could bail out as soon as the first nsIMsgCompConvertible::No element is hit. We could do this optimization. Can you file a bug for it?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: