245520 - only auto-remember "html" in ab for "send format", not "plain"

Status: RESOLVED INVALID

(MailNews Core :: Composition, defect)

Description

[Brian Ryner (not reading)]

fix "send format" stuff

I think I made things worse with my changes for bug #44494 and bug #243133.

I better go check and make sure this isn't on 1.7 branch.

here are some of the problems mscott and I have seen:

1)  even if the pref is "ask me" we don't ask when downgrading to plain text.
ask me should always ask me, as if the use has "ask", then this is silent data.
 if someone wants silently dataloss, they would choose "send plain text", not "ask".

2)  we remember the plain / html, and I think we might be remembering plain in
the scenario where I send an html email without any markup, so it gets silently
downgraded to plain, and then we remember plain.  in that scenario, we should
"remember" html in the AB.

3)  speaking of html -> text, I've seen a buggy conversions.  I keep my eye out,
and log bugs.

4)  on a related note, see my recent bug about "do send both" by default.  (bug #)

I'll fix this for 1.8 a2 / tbird aviary branch

Comment 1

[(not reading, please use [e-mail] instead)]

brian:  first #245519, now this?  I've had just about enough of you.

Comment 2

[Ben Bucksch (:BenB)]

> even if the pref is "ask me" we don't ask when downgrading to plain text.
> ask me should always ask me, as if the use has "ask", then this is silent
> data [loss}

In which cases, exactly?

If there's no formatting whatsoever (not even a <li>), there's no dataloss, so
we automatically convert to plaintext (format=flowed to be precise) without
asking the user. Why bother him? The "ask" mode is supposed to do the sensible
thing when detectable, i.e. "ask if needed", not "always ask". If you want to
have fine and definite control per mail, you can use the Options|Format menu in
the Composer.

> I think we might be remembering plain ... where ... it gets silently
> downgraded to plain, and then we remember plain. ...
> in that scenario, we should "remember" html in the AB.

That would be a bug (both). We should only ever remember the decision, if the
user actually made a choice, not when autodetecting based on a concrete composed
mail.

> speaking of html -> text, I've seen a buggy conversions.  I keep my eye
> out, and log bugs.

Please test carefully and cc me on any bugs.

> I think I made things worse

Generally, please compare with old Mozilla releases (1.4 or even 1.0), I don't
think much was *supposed* to change between then and now (apart from bug 44494).

Comment 3

[Scott MacGregor]

one of the cases where we silently downconvert to plain text when we shouldn't:

i insert inline images into HTML mail compose but don't have any other "HTML"
elements that would keep us from down converting. That one bites me all the
time. The user at the other end gets my text but none of my inline images
because we sent as plain text. 

Comment 4

[Ben Bucksch (:BenB)]

> silently downconvert to plain text  ... insert inline images into HTML

Can't reproduce with 1.4.x. Please file a bug (add reference or cc me) with
exact reproduction steps. Relevant code: nsMsgCompose::TagConvertible,
nsIMsgCompConvertible.

Comment 5

[Ben Bucksch (:BenB)]

> Can't reproduce with 1.4.x.

neither with trunk

Comment 6

[[e-mail]]

I don't know if this is relevant, or if I should open a new bug on the following
issue: if I send someone some unformatted email their addressbook card gets set
to prefer plain text, without any input from me! And while I mention it I quite
like confirmation dialogs so I don't like the way this one turns itself off :-(

Comment 7

[Mike Cowperthwaite]

(In reply to comment #3)
> one of the cases where we silently downconvert to plain text when we
> shouldn't:
> 
> i insert inline images into HTML mail compose but don't have any other "HTML"
> elements that would keep us from down converting. That one bites me all the
> time. The user at the other end gets my text but none of my inline images
> because we sent as plain text. 

Bug 180997.

"Silent loss of image" happens when the recipient is in the AB as "prefers 
plain."  If the recipient is unlisted or has an unknown pref, Moz acts per the 
setting in Prefs|Mail|Send Format -- if that happens to be "Convert to plain" 
the image is lost silently then, too.

This also affects other types of file when embedded -- e.g. drag-n-drop a PDF 
into the HTML Compose message body.


(In reply to comment #6)
> I don't know if this is relevant, or if I should open a new bug on the
> following issue: if I send someone some unformatted email their addressbook
> card gets set to prefer plain text, without any input from me! And while I
> mention it I quite like confirmation dialogs so I don't like the way this one
> turns itself off :-(

As I noted in bug 44494 comment 35, I've been unable to see any automatic 
changes to an existing AB entry, using the trunk.  I just tried again with 
1.8a2-060208/Win2k, still not.  Was that change in TB only?

Comment 8

[(not reading, please use [e-mail] instead)]

accepting, I hope to fix this for 1.8a2 and tbird 0.7

Comment 9

[(not reading, please use [e-mail] instead)]

the first part of the problem here is that even if we are doing html mail
compose, if we silently downgrade to plain text (becaue there was no html in the
message), we are remembering "plain text".

here's a stack to that scenario:

 	addrbook.dll!nsAbAddressCollecter::CollectAddress(const char *
aAddress=0x04e48220, int aCreateCard=1, unsigned int aSendFormat=1)  Line 98	C++
>	msgcompo.dll!nsMsgComposeAndSend::DeliverFileAsMail()  Line 3434	C++
 	msgcompo.dll!nsMsgComposeAndSend::DeliverMessage()  Line 3345 + 0xf	C++
 	msgcompo.dll!nsMsgComposeAndSend::GatherMimeAttachments()  Line 1210 + 0xf	C++
 	msgcompo.dll!nsMsgAttachmentHandler::UrlExit(unsigned int status=0, const
unsigned short * aMsg=0x00000000)  Line 1241 + 0x29	C++
 	msgcompo.dll!FetcherURLDoneCallback(unsigned int aStatus=0, const char *
aContentType=0x04e73c90, const char * aCharset=0x00000000, int totalSize=253,
const unsigned short * aMsg=0x00000000, void * tagData=0x04e484c8)  Line 482 +
0x10	C++
 	msgcompo.dll!nsURLFetcher::OnStopRequest(nsIRequest * request=0x04e49380,
nsISupports * ctxt=0x00000000, unsigned int aStatus=0)  Line 318 + 0x32	C++
 	docshell.dll!nsDocumentOpenInfo::OnStopRequest(nsIRequest *
request=0x04e49380, nsISupports * aCtxt=0x00000000, unsigned int aStatus=0) 
Line 360	C++
 	necko.dll!nsFileChannel::OnStopRequest(nsIRequest * req=0x04e49ba8,
nsISupports * ctx=0x00000000, unsigned int status=0)  Line 577	C++
 	necko.dll!nsInputStreamPump::OnStateStop()  Line 506	C++
 	necko.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream *
stream=0x04e4972c)  Line 340 + 0xb	C++
 	xpcom.dll!nsInputStreamReadyEvent::EventHandler(PLEvent * plevent=0x04e49c24)
 Line 119	C++
 	xpcom.dll!PL_HandleEvent(PLEvent * self=0x04e49c24)  Line 692 + 0xa	C
 	xpcom.dll!PL_ProcessPendingEvents(PLEventQueue * self=0x049a2b88)  Line 627
+ 0x9	C
 	xpcom.dll!_md_EventReceiverProc(HWND__ * hwnd=0x003903e4, unsigned int
uMsg=49483, unsigned int wParam=0, long lParam=77212552)  Line 1433 + 0x9	C
 	user32.dll!77d43a50() 	
 	user32.dll!77d43b1f() 	
 	user32.dll!77d43ed1() 	
 	user32.dll!77d43d79() 	
 	user32.dll!77d43ddf() 	
 	gkwidget.dll!nsAppShell::DispatchNativeEvent(int aRealEvent=1, void *
aEvent=0x0205a9b0)  Line 221	C++
 	appshell.dll!nsXULWindow::ShowModal()  Line 385	C++
 	appshell.dll!nsWebShellWindow::ShowModal()  Line 1103	C++
 	appshell.dll!nsContentTreeOwner::ShowAsModal()  Line 467	C++
 	embedcomponents.dll!nsWindowWatcher::OpenWindowJS(nsIDOMWindow *
aParent=0x039a04cc, const char * aUrl=0x049a1db0, const char * aName=0x00000000,
const char * aFeatures=0x0012ca60, int aDialog=1, unsigned int argc=1, long *
argv=0x04c173b4, nsIDOMWindow * * _retval=0x0012cacc)  Line 782	C++
 	gklayout.dll!GlobalWindowImpl::OpenInternal(const nsAString & aUrl={...},
const nsAString & aName={...}, const nsAString & aOptions={...}, int aDialog=1,
long * argv=0x04c173a8, unsigned int argc=4, nsISupports *
aExtraArgument=0x00000000, nsIDOMWindow * * aReturn=0x0012ce4c)  Line 4641 +
0x8c	C++
 	gklayout.dll!GlobalWindowImpl::OpenDialog(nsIDOMWindow * * _retval=0x0012ce4c)
 Line 3401 + 0x35	C++
 	xpcom.dll!XPTC_InvokeByIndex(nsISupports * that=0x039a04ec, unsigned int
methodIndex=16, unsigned int paramCount=1, nsXPTCVariant * params=0x0012ce4c)
 Line 102	C++
 	xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...},
XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 2027 + 0x1e	C++
 	xpc3250.dll!XPC_WN_CallMethod(JSContext * cx=0x039e0b60, JSObject *
obj=0x03942f78, unsigned int argc=4, long * argv=0x04c173a8, long *
vp=0x0012d120)  Line 1287 + 0xe	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x039e0b60, unsigned int argc=4, unsigned
int flags=0)  Line 1281 + 0x20	C
 	js3250.dll!js_Interpret(JSContext * cx=0x039e0b60, long * result=0x0012db8c) 
Line 3375 + 0xf	C
 	js3250.dll!js_Invoke(JSContext * cx=0x039e0b60, unsigned int argc=1, unsigned
int flags=2)  Line 1301 + 0xd	C
 	js3250.dll!js_InternalInvoke(JSContext * cx=0x039e0b60, JSObject *
obj=0x03bfbfc0, long fval=76080584, unsigned int flags=0, unsigned int argc=1,
long * argv=0x0012de28, long * rval=0x0012de30)  Line 1378 + 0x14	C
 	js3250.dll!JS_CallFunctionValue(JSContext * cx=0x039e0b60, JSObject *
obj=0x03bfbfc0, long fval=76080584, unsigned int argc=1, long * argv=0x0012de28,
long * rval=0x0012de30)  Line 3631 + 0x1f	C
 	gklayout.dll!nsJSContext::CallEventHandler(JSObject * aTarget=0x03bfbfc0,
JSObject * aHandler=0x0488e5c8, unsigned int argc=1, long * argv=0x0012de28,
long * rval=0x0012de30)  Line 1264 + 0x21	C++
 	gklayout.dll!nsJSEventListener::HandleEvent(nsIDOMEvent * aEvent=0x04b63848) 
Line 174 + 0x2d	C++
 	gklayout.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct *
aListenerStruct=0x03dc9838, nsIDOMEvent * aDOMEvent=0x04b63848,
nsIDOMEventTarget * aCurrentTarget=0x04c24378, unsigned int aSubType=4, unsigned
int aPhaseFlags=2)  Line 1460 + 0x14	C++
 	gklayout.dll!nsEventListenerManager::HandleEvent(nsIPresContext *
aPresContext=0x03b3ca30, nsEvent * aEvent=0x0012ef9c, nsIDOMEvent * *
aDOMEvent=0x0012ee70, nsIDOMEventTarget * aCurrentTarget=0x04c24378, unsigned
int aFlags=2, nsEventStatus * aEventStatus=0x0012f48c)  Line 1555	C++
 	gklayout.dll!nsXULElement::HandleDOMEvent(nsIPresContext *
aPresContext=0x03b3ca30, nsEvent * aEvent=0x0012ef9c, nsIDOMEvent * *
aDOMEvent=0x0012ee70, unsigned int aFlags=2, nsEventStatus *
aEventStatus=0x0012f48c)  Line 2788	C++
 	gklayout.dll!nsXULElement::HandleDOMEvent(nsIPresContext *
aPresContext=0x03b3ca30, nsEvent * aEvent=0x0012ef9c, nsIDOMEvent * *
aDOMEvent=0x0012ee70, unsigned int aFlags=2, nsEventStatus *
aEventStatus=0x0012f48c)  Line 2805 + 0x38	C++
 	gklayout.dll!nsXULElement::HandleDOMEvent(nsIPresContext *
aPresContext=0x03b3ca30, nsEvent * aEvent=0x0012ef9c, nsIDOMEvent * *
aDOMEvent=0x0012ee70, unsigned int aFlags=2, nsEventStatus *
aEventStatus=0x0012f48c)  Line 2805 + 0x38	C++
 	gklayout.dll!nsXULElement::HandleDOMEvent(nsIPresContext *
aPresContext=0x03b3ca30, nsEvent * aEvent=0x0012ef9c, nsIDOMEvent * *
aDOMEvent=0x0012ee70, unsigned int aFlags=7, nsEventStatus *
aEventStatus=0x0012f48c)  Line 2805 + 0x38	C++
 	gklayout.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0012ef9c,
nsIView * aView=0x00000000, unsigned int aFlags=1, nsEventStatus *
aStatus=0x0012f48c)  Line 6029 + 0x2c	C++
 	gklayout.dll!PresShell::HandleEventWithTarget(nsEvent * aEvent=0x0012ef9c,
nsIFrame * aFrame=0x045a9670, nsIContent * aContent=0x03dcfd28, unsigned int
aFlags=1, nsEventStatus * aStatus=0x0012f48c)  Line 5984 + 0x16	C++
 	gklayout.dll!nsEventStateManager::CheckForAndDispatchClick(nsIPresContext *
aPresContext=0x03b3ca30, nsMouseEvent * aEvent=0x0012f6b0, nsEventStatus *
aStatus=0x0012f48c)  Line 2962 + 0x42	C++
 	gklayout.dll!nsEventStateManager::PostHandleEvent(nsIPresContext *
aPresContext=0x03b3ca30, nsEvent * aEvent=0x0012f6b0, nsIFrame *
aTargetFrame=0x045a9670, nsEventStatus * aStatus=0x0012f48c, nsIView *
aView=0x045ac038)  Line 1969 + 0x17	C++
 	gklayout.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0012f6b0,
nsIView * aView=0x045ac038, unsigned int aFlags=1, nsEventStatus *
aStatus=0x0012f48c)  Line 6081 + 0x34	C++
 	gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x045ac038, nsGUIEvent *
aEvent=0x0012f6b0, nsEventStatus * aEventStatus=0x0012f48c, int aForceHandle=0,
int & aHandled=1)  Line 5922 + 0x19	C++
 	gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x045ac038, nsGUIEvent
* aEvent=0x0012f6b0, int aCaptured=0)  Line 2212	C++
 	gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0012f6b0,
nsEventStatus * aStatus=0x0012f588)  Line 1952 + 0x14	C++
 	gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012f6b0)  Line 79	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012f6b0,
nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1060 + 0xa	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012f6b0) 
Line 1081	C++
 	gkwidget.dll!nsWindow::DispatchMouseEvent(unsigned int aEventType=301,
unsigned int wParam=0, nsPoint * aPoint=0x00000000)  Line 5204 + 0x15	C++
 	gkwidget.dll!ChildWindow::DispatchMouseEvent(unsigned int aEventType=301,
unsigned int wParam=0, nsPoint * aPoint=0x00000000)  Line 5457	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=514, unsigned int
wParam=0, long lParam=524451, long * aRetValue=0x0012fbd8)  Line 3953 + 0x1c	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000f01da, unsigned int
msg=514, unsigned int wParam=0, long lParam=524451)  Line 1342 + 0x1b	C++
 	user32.dll!77d43a50() 	
 	user32.dll!77d43b1f() 	
 	user32.dll!77d43d79() 	
 	ntdll.dll!77f944a8() 	
 	user32.dll!77d43fd4() 	
 	user32.dll!77d43ddf() 	
 	gkwidget.dll!nsAppShell::Run()  Line 135	C++
 	appshell.dll!nsAppShellService::Run()  Line 524	C++
 	mozilla.exe!main1(int argc=4, char * * argv=0x002a5790, nsISupports *
nativeApp=0x009d3708)  Line 1302 + 0x20	C++
 	mozilla.exe!main(int argc=4, char * * argv=0x002a5790)  Line 1779 + 0x25	C++
 	mozilla.exe!mainCRTStartup()  Line 398 + 0x11	C
 	kernel32.dll!77e814c7() 	
 	ntdll.dll!77f944a8() 	

Comment 10

[Ben Bucksch (:BenB)]

Seth, from what I see, the fix for bug 44494 was indeed checked into Mozilla
1.7. I filed a separate bug about this problem (number 2 in the initial
description and  comment 6), and I nominated it for 1.7.

Comment 11

[Ben Bucksch (:BenB)]

Looking over the other problems mentioned, they all seem to be invalid,
unfounded or otherwise filed, so close this bug?

Comment 12

[(not reading, please use [e-mail] instead)]

morphing the bug.  patch coming....

Comment 13

[(not reading, please use [e-mail] instead)]

Attachment: patch

if the user choose html or both in the "ask format" dialog, we will auto
remember "html" in the ab.

if they choose plain, or if we silently convert to plain because the message
body is "convertible" we will not remember that, as it is causing problems,
namely silent conversion.

hard-core "plain text" users can still set preferred formats to "plain" in the
ab.

Comment 14

[Scott MacGregor]

Comment on attachment 150780 [details] [diff] [review]
patch

excellent. Thanks for changing this seth.

Comment 15

[(not reading, please use [e-mail] instead)]

fixed on the trunk (1.8a2) and aviary 1.0 branch.

for some spin off issues, see:

bug #246758 and bug #246757

Comment 16

[Ben Bucksch (:BenB)]

Seth, I think this is not a good fix. If you remember the choice for HTML, from
then on eternally sending HTML to this recipient, but keep asking over and over,
if the user choice plaintext, this is not only unexpected for the user, but can
be interpreted as "nagging" users until they choose HTML, at which point we stop
and send HTML forever. This doesn't make much sense to me.

A proper fix would be to set the AB setting from the dialog, and only if a
"remember" checkbox is set.
If you don't want to implement that for now, please back it out completely until
then. REOPENing.

Comment 17

[(not reading, please use [e-mail] instead)]

> Seth, from what I see, the fix for bug 44494 was indeed checked into Mozilla
> 1.7

ben, I don't think this is part of 1.7.  what make you think that?

Comment 18

[Ben Bucksch (:BenB)]

I'm sorry, I looked in the wrong tree.

(I still think this is not the proper fix.)

Comment 19

[Ben Bucksch (:BenB)]

*** Bug 246752 has been marked as a duplicate of this bug. ***

Comment 20

[(not reading, please use [e-mail] instead)]

invalid.  

see bug #44494.  for 1.8a2 / tbird 0.7, we don't remember either.

maybe fore 1.8 beta, we will add a checkbox and remember this properly.