Closed Bug 231701 Opened 17 years ago Closed 15 years ago

format=flowed DelSp=yes not supported (RFC 3676)

Categories

(MailNews Core :: MIME, enhancement)

1.0 Branch
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rst, Assigned: Bienvenu)

References

(Blocks 3 open bugs, )

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 3 obsolete files)

User-Agent:       Opera/7.22 (Windows NT 5.1; U)  [en]
Build Identifier: Mozilla Thunderbird 0.4 (20031205)

In http://www.ietf.org/internet-drafts/draft-gellens-format-bis-04.txt 
format=flowed DelSp="yes" is defined. It says:
  If the line is flowed and DelSp is "yes", the trailing space
  immediately prior to the line's CRLF is logically deleted.

Mozilla/Thunderbird does not support this. In the case a message from a mail 
client supporting (and using) this is received, the leading space is not 
logically deleted and there are 2(!) spaces displayed prior to the (omitted) 
CRLF.

Reproducible: Always

Steps to Reproduce:
1. Receive a message from a client supporting format=flowed DelSp="yes" (e.g. 
Opera 7.50)
2. See that there are 2 spaces displayed instead of one where the CRLF is
Summary: format=flowed DelSp="yes" not supported → format=flowed DelSp=yes not supported
this is not thunderbird specific. It belongs in Mozilla Mail & News
Assignee: mscott → sspitzer
Component: Mail Window Front End → MIME
Product: Thunderbird → MailNews
QA Contact: stephend
Version: unspecified → 1.0 Branch
DUPE of Bug 26734
And Bug 193238 is our Japanese comlaint on replying Japanese format=flowed Mail
created by Hot Mail - folds at no space position(invalid from point of current
RFC view) and does not insert a space because Hot Mail does not support
DelSp=yes(currently valid). 

Please note that DelSp=yes is still Draft, not Standard yet.
Opera 7's action of inserting a space for format=flowed is too early.
If testing of new starndard is the purpuse, DelSp=yes support should be optional
but Opera 7(M2) always enables it.
This is M2's fault and many users are aware that(exept Opera developers).

However, I also think Mozilla's DelSp=yes support in display is good option for
us because some Mailers such as M2 have began to implement DelSp=yes support,
although DelSp=yes on mail creation should be inhibited on Mozilla, even on
replying, until Draft becomes Standard.

Reporter, please close this bug as DUPE of Bug 26734 and post comments to it.
And please ask Opera developers to make DelSp=yes support optional until
DelSp=yes becomes Starndard.
Apple Mail has also used delsp for some time, not just Opera.
@ Comment #2
Is it really a dupe? Bug 26734 is about the handling of line breaks for Japanese 
texts, also Bug 193238. They seem to be much more special than this one, don't 
they?

Maybe delsp=yes is a solution for the issues described in the two other bugs. I 
don't know how to handle that --> you decide! :-)
To Comment #4 :

Bug 193238 is only an example of bad result when format=flowed is applied on
Japanese mails without DelSp=yes enhancement.
It should be closed as DUPE of Bug 26734 or INVALID(Hot Mail's fault), I believe.

Bug 26734 is the dicussion of enhancement of RFC 2646 and the request for
Mozilla's support of it when new RFC is defined.
And http://www.ietf.org/internet-drafts/draft-gellens-format-bis-04.txt is the
draft for enhancement of RFC 2646, even though the final change will be new
DelSp=yes only.
Since enhancement of RFC 2646 is based on requirements of Asian launguages, Bug
26734 refers to such languages only.
But enhancement of RFC 2646, DelSp=yes, is independent from languages.

Therefore DelSp=yes suppot request is included in Bug 26734.
But bug 26734 is long...
Splitting DelRc=yes support request is good idea?

Anyway, pressure form other than Asia will be required to accelerate development
of new RFC 2646 support - DelSp=yes support.
René Stach, add comments to Bug 26734 from Europe, please :-)
Since DelSpc=yes/no is a parameter of format=flowed, I think people who discuss
on it should know about Bug 168420, although this has no relation to DelSp=yes
support itlsef.
> Bug 168420 We need Format=Flowed Evangelization
I hope such Evangelization in Japanese will not be required when DelSp=yes will
be supoorted.
*** Bug 264396 has been marked as a duplicate of this bug. ***
Product: MailNews → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true
Change QA contact to Momoi-san because QA contact of Bug 26734 is he.
QA Contact: stephend → momoi
*** Bug 272694 has been marked as a duplicate of this bug. ***
Summary: format=flowed DelSp=yes not supported → format=flowed DelSp=yes not supported (RFC 3676)
This bug is sound.

Even if DelSp is currently draft, it should be supported for *reading* messages.

The reason is that several popular mailers are already using DelSp.

In fact most mailers using DelSp seem to be using it for *all* messages, even
though it is unnecessary in most cases.

It seems to be a case of lazy programming by 20-year old computer kids: "DelSp
will cover all cases, so we'll just turn it on all the time".  How about
analysing the message that is about to be sent and turning on the technologies
necessary to make it be represented correctly, kids?

If, on a given message, you only ever inserted a soft newline where there was
*already* a space, then you don't need to enable DelSp for that message.

Ranting aside, Mozilla/Thunderbird should at least be supporting DelSp for
*reading* messages.
Change "assigned to" to David, since Seth, default assinged to person, is "not
reading bugmail" these months.
David, set appropriate priority and dispatch to appropriate(and available, not
too busy) person, please.
Assignee: sspitzer → bienvenu
Depends on: 26734
This bug doesn't depend on Bug 26734 because it would be possible to add
functionality for supporting DelSp=yes for *reading* without fixing the CJK line
breaking issue. 

If anything, that bug depends on this bug, because if *generation* of messages
which use DelSp=yes is used in the addressing of Bug 26734, then support for
*interpretation* of DelSp will also be required (otherwise it won't interoperate
with itself).

So I am moving Bug 26734 from "depends on" to "blocks".
Blocks: 26734
No longer depends on: 26734
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [good first bug]
Blocks: Persian
Blocks Bugzilla Bug 285718 - Meta Bug: Tracking bug for Persian 	

Because:  Lake of line-break/paragraph-separator detection cause many BiDi
problems, i.e. mail client fails to auto-align paragraph.
No longer blocks: Persian
Blocks: Persian
Blocks: 137995
Blocks: 218823
This patch seems to work on my system. Not a great fix, but maybe good enough
to last until a more thorough rewrite of format=flowed.

I am not really a developer, so the patch will probably need some tweaking by
someone who is. Feel free to pick it up.
Attachment #187914 - Flags: review?(bienvenu)
thx for the patch. I'm not a mime expert, but I think you could do without the
new Initialize method and just init delSp here:

http://lxr.mozilla.org/seamonkey/source/mailnews/mime/src/mimetpfl.cpp#123

Or, can you just do this?:

instead of:
+    if (content_type_delsp && !nsCRT::strcasecmp(content_type_delsp, "yes"))
+      ((MimeInlineTextPlainFlowed *)obj)->delSp = PR_TRUE;

just do ((MimeInlineTextPlainFlowed *)obj)->delSp = content_type_delsp &&
!nsCRT::strcasecmp(content_type_delsp, "yes"));

second version of patch with changes suggested by bienvenu
Attachment #187914 - Attachment is obsolete: true
Attachment #187948 - Flags: review?(bienvenu)
Comment on attachment 187948 [details] [diff] [review]
second version of patch with changes suggested by bienvenu

cool, thanks. Looks good to me. I'd also suggest using PR_Free instead of
PR_FREEIF because PR_Free handles null, and there's no reason to null out the
local vars because they're about to go out of scope anyway, but sometimes
people are resistant to that suggestion.
Attachment #187948 - Flags: superreview?(ducarroz)
Attachment #187948 - Flags: review?(bienvenu)
Attachment #187948 - Flags: review+
Attached patch use PR_Free instead of PR_FREEIF (obsolete) — Splinter Review
use PR_Free instead of PR_FREEIF
Attachment #187948 - Attachment is obsolete: true
Attachment #187955 - Flags: superreview?(ducarroz)
Attachment #187955 - Flags: review?(bienvenu)
Attachment #187955 - Flags: review?(bienvenu) → review+
This version of the patch checks for DelSp=yes in the parse_begin method rather
than when the object is created. This way may be slightly cleaner.

If you prefer this patch, please mark the previous patch (attachment 187955 [details] [diff] [review])
obsolete (and vice-versa).
Attachment #188062 - Flags: superreview?(ducarroz)
Attachment #188062 - Flags: review?(bienvenu)
Comment on attachment 188062 [details] [diff] [review]
alternative version of patch

sure, sounds good.
Attachment #188062 - Flags: review?(bienvenu) → review+
Attachment #187955 - Attachment is obsolete: true
Attachment #187955 - Flags: superreview?(ducarroz)
Attachment #187955 - Flags: review+
Flags: blocking-aviary1.1?
Flags: blocking-aviary1.1? → blocking1.8b4?
Comment on attachment 188062 [details] [diff] [review]
alternative version of patch

Looks good, SR=ducarroz
(sorry for the delay, I am on vacation)
Attachment #188062 - Flags: superreview?(ducarroz) → superreview+
Attachment #188062 - Flags: approval1.8b4?
Attachment #188062 - Flags: approval1.8b4? → approval1.8b4+
fix checked in, thx, James.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Great; thank you.
Flags: blocking1.8b4?
Attachment #187914 - Flags: review?(bienvenu)
Attachment #187948 - Flags: superreview?(ducarroz)
BTW: Delsp=yes seems to cause (at least in certain cases) a assertion (nsTDependentString must wrap only null-terminated strings: 'mData[mLength] == 0'), see Bug 318886 if someone wants to take a look at this.
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.