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)
MailNews Core
Composition
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)
(deleted),
patch
|
aceman
:
review+
Paenglab
:
ui-review+
|
Details | Diff | Splinter Review |
+++ 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
Assignee | ||
Comment 1•7 years ago
|
||
Let's fix this.
Attachment #8890141 -
Flags: ui-review?(richard.marti)
Attachment #8890141 -
Flags: review?(acelists)
Comment 2•7 years ago
|
||
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
Updated•7 years ago
|
Target Milestone: --- → Thunderbird 56.0
Assignee | ||
Comment 6•7 years ago
|
||
Thanks everyone! Great teamwork, great turnaround time! :)
We've really gone a long way to contain the HTML-eating behaviour of Auto-Downgrade.
Assignee | ||
Comment 7•7 years ago
|
||
(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.
(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.
Description
•