78727 - Mail Compose: Options: Rewrap not working properly

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P2)

Description

[jglick]

Windows 2001050204 build. Mail Compose window, Options menu, the "Rewrap" menu 
item does not appear to be working correctly.  It is always enabled and 
selecting it doesn't seem to have an effect.

According to bug 50311 Rewrap should be:

1.Enabled only in plain mode and disabled in HTML mode.
2.Enable only when the cursor is in the message body.

3.What exactly is "Rewrap" supposed to do?

Comment 1

[Jean-Francois Ducarroz]

reassign to varada

Comment 2

[Eric Hedström]

With win2000 win32-installer 2001-0514-04 selecting rewrap when composing email 
in plain text mode, mozilla hangs and consumes all the CPU cycles it can.

Comment 3

[scottputterman]

moving to 0.9.3.  Let's remove it.

Comment 4

[esther]

Can't be fixed at this time removing the +, logged a new bug (86998)to remove
the menu item

Comment 5

[scottputterman]

moving to future milestone.

Comment 6

[Akkana Peck]

What isn't working?  "Not working properly" isn't a very clear bug report.

There's one report of a hang.  If anyone else can reproduce the hang, it would
be useful to put a debugger on it and know where it was hanging.

Rewrap is only relevant in plaintext mail.  What it does: rewraps text
(particularly quoted text) which is wrapped incorrectly.  I.e. someone sends you
a message that's all on one line with no linebreaks, you reply to it, now you
have one "> " character followed by the entire message.  Not very easy to edit
or read., and it's very cumbersome to edit such lines by hand to put linebreaks
and "> " symbols in the right place (and there's a risk of getting it wrong and
having the message then rewrap at the wrong place).  Hence, there's rewrap to
let the mailer do it automatically.

This should be familiar to anyone who uses plaintext mail on a regular basis. 
(I encounter messages with improper wrapping almost every day.)

Comment 7

[Ninoschka Baca]

I can reproduce the hang when replying to a message in plain text mode, then 
selecting Options|Rewrap. The Option menus remains on screen with Rewrap 
highlighted, I can't switch to any other Mojo windows. Have to Ctrl+Alt+Del to 
the End Task. This was also tried on a debug build but a hang did not occur.

Comment 8

[Akkana Peck]

The hang doesn't happen on a linux opt build from 6/25 (unless it's specific to
the particular message being replied to).  Bummer, must be platform specific
(something to do with line breaks, perhaps).  Does it hang if you select part of
the message then do rewrap?

Comment 9

[Ninoschka Baca]

I reproduced the hang on linux and mac using todays builds (06/27/2001).

Here is a stack trace from the Mac:

Calling chain using A6/R1 links
  Back chain  ISA  Caller
  00000000    PPC  0B1B1840  
  0CDD5B80    PPC  0B1A1C8C  
  0CDD5B20    PPC  0B1A1394  
  0CDD5840    PPC  0E2E98C0  NSGetModule+0181C
  0CDD5800    PPC  0E2B55AC  nsMacMessageSink::IsRaptorWindow(GrafPort*)+0141C
  0CDD57B0    PPC  0E2B5C24  nsMacMessageSink::IsRaptorWindow(GrafPort*)+01A94
  0CDD5760    PPC  0E2B60DC  nsMacMessageSink::IsRaptorWindow(GrafPort*)+01F4C
  0CDD5710    PPC  0E2B63A8  nsMacMessageSink::IsRaptorWindow(GrafPort*)+02218
  0CDD5610    PPC  0E2B6CC4  nsMacMessageSink::IsRaptorWindow(GrafPort*)+02B34
  0CDD54C0    PPC  0E2B6EAC  nsMacMessageSink::IsRaptorWindow(GrafPort*)+02D1C
  0CDD5470    PPC  0E2B4044  nsMacMessageSink::DispatchMenuCommand(EventRecord&, 
long, GrafPo
rt*)+00040
  0CDD5420    PPC  0E2AFED8  NSGetFactory+06FA4
  0CDD53E0    PPC  0E2B0B10  NSGetFactory+07BDC
  0CDD5330    PPC  0E2A5C0C  
  0CDD52F0    PPC  0E2A5B68  
  0CDD52A0    PPC  0E2C8708  Repeater::DoIdlers(const EventRecord&)+034EC
  0CDD51B0    PPC  0E2CC794  Repeater::DoIdlers(const EventRecord&)+07578
  0CDD4BE0    PPC  0E2D06DC  Repeater::DoIdlers(const EventRecord&)+0B4C0
  0CDD4BA0    PPC  0E2D0958  Repeater::DoIdlers(const EventRecord&)+0B73C
  0CDD4990    PPC  0DEA4C14  NS_NewStyleContext(nsIStyleContext**, 
nsIStyleContext*, nsIAtom*
, nsIRuleNode*, nsIPresContext*)+42984
  0CDD43F0    PPC  0DD27C04  NS_NewHTMLCSSStyleSheet(nsIHTMLCSSStyleSheet**)+
09768
  0CDD4000    PPC  0DD2588C  NS_NewHTMLCSSStyleSheet(nsIHTMLCSSStyleSheet**)+
073F0
  0CDD3DC0    PPC  0E0E3B34  nsGetInterface::~nsGetInterface()+11028
  0CDD3C50    PPC  0E0CE69C  
  0CDD3B90    PPC  0E346F3C  JS_CallFunctionValue+00028
  0CDD3B50    PPC  0E3604AC  js_Invoke+008B4
  0CDD3A90    PPC  0E36028C  js_Invoke+00694
  0CDD3980    PPC  0E368128  js_Invoke+08530
  0CDD36C0    PPC  0E360234  js_Invoke+0063C
  0CDD35B0    PPC  0E3274C8  NSGetModule+0D058
  0CDD34E0    PPC  0E322D1C  NSGetModule+088AC
  0CDD3190    PPC  0E3EBFE0  XPTC_InvokeByIndex+0000C
  0CDD3150    PPC  0E3EC0C0  _XPTC_InvokeByIndex+000C8
  0CDD30A8    PPC  0E3ED5A8  nsXPTCStubBase::Sentinel4()+00078
  0CDD2FF8    PPC  0E3EC534  PrepareAndDispatch+0042C
  0CDD2ED8    PPC  0E31B7F8  NSGetModule+01388
  0CDD2E98    PPC  0E31D88C  NSGetModule+0341C
  0CDD28C8    PPC  0E36028C  js_Invoke+00694
  0CDD27B8    PPC  0E368128  js_Invoke+08530
  0CDD24F8    PPC  0E360234  js_Invoke+0063C
  0CDD23E8    PPC  0E3274C8  NSGetModule+0D058
  0CDD2318    PPC  0E322D1C  NSGetModule+088AC
  0CDD1FC8    PPC  0E3EBFE0  XPTC_InvokeByIndex+0000C
  0CDD1F88    PPC  0E3EC0C0  _XPTC_InvokeByIndex+000C8
  0CDD1EE0    PPC  0DA0C914  nsGetInterface::~nsGetInterface()+09768
  0CDD1E70    PPC  0DA4C748  NSGetModule+3562C
 Closing log

Comment 10

[Ninoschka Baca]

On the Mac I tried a couple of more scenarios:

Selected a message in the thread pane, select Reply button:
1. Options|Spell Checking..., is ok
I also tried other menu items such as Addresses...,Priority|Low, and they are 
also ok.

2. Type one character in the body, select Options|Rewrap and no hang!

So it appears that the hang only happens when no text has been entered and 
Options|Rewrap is selected.

Comment 11

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

I can reproduce this.

on my linux console, I get this:

###!!! Break: at file ../../dist/include/nsStringIterator.h, line 184
###!!! ASSERTION: Infinite loop: can't advance (backward) a reading iterator
beyond the end of a string: 'one_hop<0', file
../../dist/include/nsStringIterator.h, line 184

over and over.

akkana, do you have a signature?  maybe that matters (since typing in the window
prevents the problem)

Comment 12

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

adding scc.  could be string related.

from nsStringIterator.h

 NS_ASSERTION(one_hop<0, "Infinite loop: can't advance (backward) a reading 
iterator beyond the end of a string");
 // perhaps I should |break| if |!one_hop|?
varada, if you are unable to reproduce this, I can help debug this tomorrow.  

let me know if you need help.

Comment 13

[Akkana Peck]

I do have a signature (plaintext).  Should I try without?  Seth and Ninoschka,
do you have one?  

This is very suggestive: the last change to nsWrapUtils.cpp was Mike's change to
use the new string APIs.  The changes themselves don't call any new tricky
string APIs, but they switch the wrapping code to use a different type of
string, which may have broken rewrapping.  I expect that rewrap didn't get
tested much after that change.

Seth, we may need your help for testing a fix, since you're the only person so
far who's managed to reproduce it on a debug build.  It's apparently very
sensitive to something in the environment; Sairuh and I haven't been able to
reproduce it at all on linux.  Any chance you could get a stack trace back to
the relevant line in nsWrapUtils.cpp or nsInternetCiter.cpp?  The assertion Seth
got is in advance(), which is never called directly from nsWrapUtils, but
perhaps it's called indirectly from one of the append() calls which were changed
when the API change was made.  Then I can grab scc and get his help figuring out
what's wrong and what needs to change.

Comment 14

[Akkana Peck]

It wasn't the signature; I turned mine off and still don't see the hang, alas.

Comment 15

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

ok, I'll start debugging and get a stack trace now.

Comment 16

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

I think cursor placement on reply has an affect.

if my cursor is set to be at the bottom, no problem.

at the top, I get the problem.  debugging on winnt now.

Comment 17

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

the hang part of this problem is bug #82031

to reproduce, do a plain text reply and have your cursor set to be at the top 
(on reply).

here's the stack trace to the assertion:

NTDLL! 77f9f9df()
nsDebug::Assertion(const char * 0x05125674 `string', const char * 0x051256dc 
`string', const char * 0x05125750 `string', int 184) line 290 + 13 bytes
nsReadingIterator<unsigned short>::advance(int -1) line 184 + 32 bytes
nsInternetCiter::Rewrap(nsInternetCiter * const 0x0548c160, const nsAString & 
{...}, unsigned int 72, unsigned int 0, int 0, nsAString & {...}) line 241
nsPlaintextEditor::Rewrap(nsPlaintextEditor * const 0x04c6e2a8, int 0) line 
1774 + 62 bytes
nsEditorShell::Rewrap(nsEditorShell * const 0x07892210, int 0) line 2621 + 27 
bytes
XPTC_InvokeByIndex(nsISupports * 0x07892210, unsigned int 43, unsigned int 1, 
nsXPTCVariant * 0x0012be98) line 139
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode 
CALL_METHOD) line 1881 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x06de05f0, JSObject * 0x0480e500, unsigned int 
1, long * 0x048f3020, long * 0x0012c0cc) line 1252 + 11 bytes
js_Invoke(JSContext * 0x06de05f0, unsigned int 1, unsigned int 0) line 807 + 23 
bytes
js_Interpret(JSContext * 0x06de05f0, long * 0x0012ce70) line 2702 + 15 bytes
js_Invoke(JSContext * 0x06de05f0, unsigned int 1, unsigned int 2) line 824 + 13 
bytes
nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJSClass * const 0x03e651d0, 
nsXPCWrappedJS * 0x07a9ae00, unsigned short 5, const nsXPTMethodInfo * 
0x02b62108, nsXPTCMiniVariant * 0x0012d3b0) line 1000 + 21 bytes
nsXPCWrappedJS::CallMethod(nsXPCWrappedJS * const 0x07a9ae00, unsigned short 5, 
const nsXPTMethodInfo * 0x02b62108, nsXPTCMiniVariant * 0x0012d3b0) line 427
PrepareAndDispatch(nsXPTCStubBase * 0x07a9ae00, unsigned int 5, unsigned int * 
0x0012d460, unsigned int * 0x0012d450) line 100 + 31 bytes
SharedStub() line 124
XPTC_InvokeByIndex(nsISupports * 0x07a9ae00, unsigned int 5, unsigned int 1, 
nsXPTCVariant * 0x0012d5e0) line 139
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode 
CALL_METHOD) line 1881 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x06de05f0, JSObject * 0x04749250, unsigned int 
1, long * 0x048f2fe8, long * 0x0012d814) line 1252 + 11 bytes
js_Invoke(JSContext * 0x06de05f0, unsigned int 1, unsigned int 0) line 807 + 23 
bytes
js_Interpret(JSContext * 0x06de05f0, long * 0x0012e5b8) line 2702 + 15 bytes
js_Invoke(JSContext * 0x06de05f0, unsigned int 1, unsigned int 2) line 824 + 13 
bytes
js_InternalInvoke(JSContext * 0x06de05f0, JSObject * 0x0480e648, long 75556456, 
unsigned int 0, unsigned int 1, long * 0x0012e790, long * 0x0012e6e0) line 896 
+ 20 bytes
JS_CallFunctionValue(JSContext * 0x06de05f0, JSObject * 0x0480e648, long 
75556456, unsigned int 1, long * 0x0012e790, long * 0x0012e6e0) line 3320 + 31 
bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x06de07a0, void * 
0x0480e648, void * 0x0480e668, unsigned int 1, void * 0x0012e790, int * 
0x0012e78c, int 0) line 941 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x06e4cc90, 
nsIDOMEvent * 0x05489154) line 139 + 57 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x06e4cc10, 
nsIDOMEvent * 0x05489154, nsIDOMEventTarget * 0x06e4cce8, unsigned int 8, 
unsigned int 7) line 1161 + 20 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x06e4edf0, 
nsIPresContext * 0x06df1a90, nsEvent * 0x0012f2b0, nsIDOMEvent * * 0x0012f15c, 
nsIDOMEventTarget * 0x06e4cce8, unsigned int 7, nsEventStatus * 0x0012f2fc) 
line 2131 + 36 bytes
nsXULElement::HandleDOMEvent(nsXULElement * const 0x06e4cce0, nsIPresContext * 
0x06df1a90, nsEvent * 0x0012f2b0, nsIDOMEvent * * 0x0012f15c, unsigned int 1, 
nsEventStatus * 0x0012f2fc) line 3635
PresShell::HandleDOMEventWithTarget(PresShell * const 0x06df9d30, nsIContent * 
0x06e4cce0, nsEvent * 0x0012f2b0, nsEventStatus * 0x0012f2fc) line 5677 + 39 
bytes
nsMenuFrame::Execute() line 1424
nsMenuFrame::HandleEvent(nsMenuFrame * const 0x04792a20, nsIPresContext * 
0x06df1a90, nsGUIEvent * 0x0012f754, nsEventStatus * 0x0012f648) line 399
PresShell::HandleEventInternal(nsEvent * 0x0012f754, nsIView * 0x054872b0, 
unsigned int 1, nsEventStatus * 0x0012f648) line 5645 + 41 bytes
PresShell::HandleEvent(PresShell * const 0x06df9d34, nsIView * 0x054872b0, 
nsGUIEvent * 0x0012f754, nsEventStatus * 0x0012f648, int 0, int & 1) line 5557 
+ 25 bytes
nsView::HandleEvent(nsView * const 0x054872b0, nsGUIEvent * 0x0012f754, 
unsigned int 8, nsEventStatus * 0x0012f648, int 0, int & 1) line 377
nsView::HandleEvent(nsView * const 0x05430410, nsGUIEvent * 0x0012f754, 
unsigned int 8, nsEventStatus * 0x0012f648, int 0, int & 1) line 350
nsView::HandleEvent(nsView * const 0x05428e50, nsGUIEvent * 0x0012f754, 
unsigned int 8, nsEventStatus * 0x0012f648, int 0, int & 1) line 350
nsView::HandleEvent(nsView * const 0x06df11d0, nsGUIEvent * 0x0012f754, 
unsigned int 28, nsEventStatus * 0x0012f648, int 1, int & 1) line 350
nsViewManager::DispatchEvent(nsViewManager * const 0x06df1370, nsGUIEvent * 
0x0012f754, nsEventStatus * 0x0012f648) line 2051
HandleEvent(nsGUIEvent * 0x0012f754) line 68
nsWindow::DispatchEvent(nsWindow * const 0x06df1094, nsGUIEvent * 0x0012f754, 
nsEventStatus & nsEventStatus_eIgnore) line 720 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f754) line 741
nsWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 4237 
+ 21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 
4486
nsWindow::ProcessMessage(unsigned int 514, unsigned int 0, long 2949239, long * 
0x0012fb98) line 3203 + 24 bytes
nsWindow::WindowProc(HWND__ * 0x0099015a, unsigned int 514, unsigned int 0, 
long 2949239) line 988 + 27 bytes
USER32! 77e13eb0()
USER32! 77e1401a()
USER32! 77e192da()
nsAppShellService::Run(nsAppShellService * const 0x00e69660) line 419
main1(int 2, char * * 0x00484190, nsISupports * 0x00000000) line 1200 + 32 bytes
main(int 2, char * * 0x00484190) line 1504 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

Comment 18

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

from nsInternetCiter.cpp, line 240:

  outPeekIter.advance(-1);

posInString is 0.

akkana / varada / ducarroz, let me know if you need more help in debugging.

Comment 19

[Akkana Peck]

Okay, here's what this used to say:

      if (aOutString.Length() > 0 && aOutString[aOutString.Length()-1] != nl)
        aOutString.Append(nl);
 
and here's the same section of code now:
      nsReadingIterator <PRUnichar> outPeekIter;
      aOutString.EndReading(outPeekIter);
      outPeekIter.advance(-1);
      if (aOutString.Length() > 0 && (*outPeekIter) != nl)
        aOutString.Append(nl);

This happened in version 1.20 when Mike changed the string APIs.

I'm not having much luck reading that string iterator code, but it looks to me
like whole enclosing clause should not be executed if aOutString is null, so
perhaps the best thing is to change the enclosing condition to include that
rather than waiting until inside the clause to check the same condition.

About to attach a patch that does that (and also gets rid of some unused
variables which were causing warnings, because I Just Can't Stand It).

Comment 20

[Akkana Peck]

Attachment: Patch: test for OutString's length first

Comment 21

[Akkana Peck]

BTW, thanks a lot, Seth, for tracking this down!

Comment 22

[scottputterman]

Do we know if this patch solves the problem?  It sounds like Seth's the only one
who has seen the bug in a debug build.  

Comment 23

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

anyone should be able to see this in any build, once they set the pref to set 
the cursor to be at the top on reply.

Comment 24

[scottputterman]

I've been told that we would have a shot of getting this on the branch and if we
can get this bug through the review process and feel confident that it works and
doens't cause regressions.  Seth, JF or Varada, can you try this when you get a
chance? 

Comment 25

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

seems like a low risk fix to me.

as always, the author (akkana) would be the best judge of the risk involved.

the first step is to hear from varada if it worked or not. 

Comment 26

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

varada is having linux machine problems.  testing this now...

Comment 27

[Akkana Peck]

The fix should be very safe.  Assuming it works, it's hard to imagine that it
could break anything.

Comment 28

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

the patch works fine.

I was able to get the inf. loop without the patch, and then try again with it
and the problem was gone.

r=sspitzer, but you probably want an editor person to review it.

Comment 29

[Akkana Peck]

Simon, can you sr this very small patch?  Two lines, plus a couple of warning fixes.

Comment 30

[Simon Fraser [no longer active]]

sr=sfraser

Comment 31

[Akkana Peck]

Looks like this is ready to go.  Want me to take this bug and check it in, or do
the mail folks want to?  Has any manager-type approved this for the branch?

Comment 32

[scottputterman]

No one has approved this for the branch.  If you can get this on the trunk first
that would help out.  I'll look into getting approval from PDT.  I'll cc you on
the mail.  thanks for looking at this.

Comment 33

[Akkana Peck]

Taking the bug, and I will check it in to the trunk.

Comment 34

[Akkana Peck]

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

Comment 35

[scottputterman]

adding vtrunk and waiting for some QA testing per PDT before checking into branch.

Comment 36

[rubydoo123]

adding nsBranch and nsBranch+, awaiting review by PDT

Comment 37

[Ninoschka Baca]

Trunk build 2001-07-10-05: WinMe, ok.
Trunk build 2001-07-10-04: Linux RH 6.2, ok.
Trunk build 2001-07-10-08: Mac 9.04, ok.

- Created a new message in Plain format, selected Options|Rewrap, no hang.
- Replied to a message in Plain format, selected Options|Rewrap, no hang.

Comment 38

[Ninoschka Baca]

trunkverified

Comment 39

[Ninoschka Baca]

I tried more scenarios on all operating systems and it appears ok. I tried basic 
messages, messages that were more complicated such as web pages attached. I was 
able to continue regular use afterwards. 

Comment 40

[lchiang]

PDT+ per email to/from PDT.
Once you mark this fixed, pls remove the vtrunk keyword.  Thanks.

Comment 41

[Akkana Peck]

Fix is now in branch as well as trunk.  Marking fixed, removing vtrunk.

Comment 42

[Sheela Ravindran]

Verified this on mac trunk only since todays builds are not available. I used 
the two scenarios described by nbaca from her comments from 2001-07-10 10:20 
Build id: 2001071208

Comment 43

[Sheela Ravindran]

verifying this bug since nbaca has already verified this on the trunk 

Branch build: 2001-07-13-06 win98
Branch build: 2001-07-13-03 Mac
Branch build: 2001-07-13-04 Linux

Comment 44

[Sheela Ravindran]

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