200545 - Caret is not visible when replying to a message

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Ere Maijala (slow)]

At least since 20030402 the caret has been invisible when replying to a message.
Possibly caused by checkin for bug 35296. Latest test using 2003040308 on WinXP.

Comment 1

[leon.zhang]

reply a message first time after open mailnews, caret can be seen.
In following replying, caret disappear.

Comment 2

[leon.zhang]

replay a mesage at first time after start mailnews, we will meet stack below:

nsCaret::DrawCaret() line 994
nsCaret::StartBlinking() line 534
nsCaret::SetCaretVisible() line 236 + 8 bytes
PresShell::SetCaretEnabled() line 3135 + 36 bytes
nsTextEditorFocusListener::Focus() line 1183          *******************
nsEventListenerManager::HandleEvent() line 1688 + 41 bytes
nsDocument::HandleDOMEvent() line 3610
nsEventStateManager::PreHandleEvent() line 496


in function: nsTextEditorFocusListener::Focus()
    mEditor->GetFlags(&flags);
    if (! (flags & nsIPlaintextEditor::eEditorDisabledMask))
    { // only enable caret and selection if the editor is not disabled 
      .....
      selCon->SetCaretEnabled(PR_TRUE);
      .....
    }

In follwoing reply, we will not go into branch which will call
selCon->SetCaretEnabled(PR_TRUE) and draw caret at the first time.

Value of mflags of mEditor may be source of this bug.

Comment 3

[leon.zhang]

nsTextEditorFocusListener::Focus()

 mEditor->GetFlags(&flags);
    if (! (flags & nsIPlaintextEditor::eEditorDisabledMask))
    { // only enable caret and selection if the editor is not disabled
          if (! (flags & nsIPlaintextEditor::eEditorReadonlyMask))
          { // only enable caret if the editor is not readonly
             ...
             selCon->SetCaretEnabled(PR_TRUE);
             ...
           }
     }

when second reply, flags have been set nsIPlaintextEditor::eEditorReadonlyMask,
when? why?

Comment 4

[leon.zhang]

the flag have been set in function:

nsMsgCompose::CloseWindow() -> ... -> nsHTMLEditor::SetFlags()

you can verify above result:
  if you do not close the windows result from replying messages, caret can be
seen during following reply. 

Comment 5

[leon.zhang]

whole process set flags of edtor when close a compose window:

nsMsgCompose::CloseWindow()
  : mRecyclingListener->OnClose();
...> gComposeRecyclingListener = {  onClose: function() }
-->  disableEditableFields();
    : gMsgCompose.editor.flags |= nsIPlaintextEditorMail.eEditorReadonlyMask;
...>  nsHTMLEditor::SetFlags()

I am not sure the true reason for current setting flag.

Comment 6

[leon.zhang]

if we comment out codes below:
gMsgCompose.editor.flags |= nsIPlaintextEditorMail.eEditorReadonlyMask;
and
gMsgCompose.editor.flags &= ~nsIPlaintextEditorMail.eEditorReadonlyMask;
in function disableEditableFields() and enableEditableFields().

we can fix this bug, but will lead to another regression:
when compose a new message, caret will appear or disappear each time.

Comment 7

[leon.zhang]

if caret can be seen in new composer window, calling sequence is:
nsTextEditorFocusListener::Focus()
...
nsTextEditorFocusListener::Blur()
...
nsTextEditorFocusListener::Focus()

------------------------------------------------------------

if caret cannot be seen in new composer window, calling sequence is:
nsTextEditorFocusListener::Focus()
...
nsTextEditorFocusListener::Blur()

Comment 8

[[e-mail]]

Can you try deleting line 237 of MsgComposeCommands.js to see if that helps?

I think it might also resolve a related bug that I have noticed recently.

Comment 9

[leon.zhang]

neil: I comment out that line; window.content.focus();
no use.

Comment 10

[leon.zhang]

Neil: 
I test your comments again based on orginal codes, reply  is ok.
but when compose a new message, caret will apear in odd times and disappear in
even times.

I think there exist some wrong in status of recycled window.

your suggesions? thx

Comment 11

[[e-mail]]

Attachment: Proposed patch

Deleting line 237 of MsgComposeCommands.js works for me, with a recent nightly
(2003040604). But while I was there I noticed that Compose Mail To (or mailto:
links without a subject) don't work for recycled windows - they try to set
focus to a disabled subject field. So I moved enableEditableFields() to fix
that too.

Comment 12

[leon.zhang]

Comment on attachment 119792 [details] [diff] [review]
Proposed patch

I test this patch.
reply/compose/forward are OK.
thx neil

Comment 13

[Cavin Song]

Comment on attachment 119792 [details] [diff] [review]
Proposed patch

r=cavin.

Comment 14

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

I want to make sure we test on win32, and test compose / reply with the cached
(and uncached) compose windows.

I should be able to do that today.

Comment 15

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

Comment on attachment 119792 [details] [diff] [review]
Proposed patch

sr=sspitzer

I tested cached X not cached, reply, new, fwd (with various cursor placements
on reply).

fix landed, thanks neil!

Comment 16

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

fixed