Caret disappears in cached compose window body

VERIFIED FIXED

Status

()

VERIFIED FIXED
14 years ago
14 years ago

People

(Reporter: mcsmurf, Assigned: neil)

Tracking

({regression})

Trunk
x86
Windows 2000
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

14 years ago
To reproduce:
1. Open MailNews Composer
2. Paste some text into subject
3. Press tab key
4. Observe that caret is invisible

Comment 2

14 years ago
I am seeing this same bug with 2005052706, seamonkey win2k. Are you sure the
regression window is correct?
Is this related?

Error: Unknown namespace prefix 'html'.  Ruleset ignored due to bad selector.
Source File: chrome://messenger/skin/messageBody.css
Line: 54
(In reply to comment #3)
> Is this related?
> 
> Error: Unknown namespace prefix 'html'.  Ruleset ignored due to bad selector.
> Source File: chrome://messenger/skin/messageBody.css
> Line: 54

Yeah, ignore this comment.

Instead, I've found a particular case that fails between 2005-05-19-05 and
2005-05-20-05:

1. Launch Mail.
2. Select a folder.
3. Select a message in the folder.
4. Click Compose.
5. Click to focus the compose message body.

Notice there's no caret at this point.

I could be way off base, but please do humor me:

Boris, could this be from bug 293914?

In particular, the following changes to nsEventStateManager.cpp:

-  if (aContent == mActiveContent) {
-    mActiveContent = nsnull;
+  if (mActiveContent &&
+      nsContentUtils::ContentIsDescendantOf(mActiveContent, aContent)) {
+    // Active is hierarchical, so set the current active to the
+    // content's parent node.
+    mActiveContent = aContent->GetParent();

I apologize in advance if I'm way off base here.
Those are very unlikely to have caused this.

Further, I can't reproduce this with either the steps in comment 0 or the steps
in comment 4, with either Seamonkey mailnews or tbird.  Both Linux debug builds
pulled at MOZ_CO_DATE="Thu Jun 2 00:44:32 CDT 2005".

Martijn, or someone else who builds on Windows, can you reproduce this?  If so,
could you possibly hunt down which checkin is the problem?
OK, so to reproduce this you have to set the recycled compose window pref to 1
(the Windows default).  Then the second and later compose windows show the problem.

The regression range is in fact 2005-05-19-05 to 2005-05-20-05.   Not sure what
in there is breaking this yet.
OK, looks like this is a regression from bug 294251.  If I comment out the
|StopInlineSpellChecker();| call that added to MsgComposeCommands.js this bug
goes away.

No idea what's going on here, really, but I do get some nice content iterator
asserts when the spellchecker restarts when we reshow the recycled compose
window.  Maybe those are relevant.
Assignee: mozeditor → nobody
Blocks: 294251
Component: Editor → MailNews: Composition
QA Contact: bugzilla
Flags: blocking-aviary1.1?
(Assignee)

Comment 8

14 years ago
(In reply to comment #7)
>OK, looks like this is a regression from bug 294251.  If I comment out the
>|StopInlineSpellChecker();| call that added to MsgComposeCommands.js this bug
>goes away.
Phew, so all we did was expose an underlying bug, that is to say, if the spell
checker is stopped either manually or (on the suite) automatically then when the
window is recycled then the cursor does not appear.

>No idea what's going on here, really, but I do get some nice content iterator
>asserts when the spellchecker restarts when we reshow the recycled compose
>window.  Maybe those are relevant.
Interestingly I run my linux build with the recycled compose window pref and it
does not assert or have any caret weirdness. It does trigger a couple of
NS_ENSURE_SUCCESS warnings at nsFilteredContentIterator.cpp:110 and
mozInlineSpellChecker.cpp:902 though. Here's a stack trace of an assert:

nsDebug::Assertion(const char * 0x01bdc7ec `string', const char * 0x01bdc7fc
`string', const char * 0x01bdc738 `string', int 965) line 109
nsContentIterator::First(nsContentIterator * const 0x00fbafd8) line 965 + 35 bytes
nsFilteredContentIterator::First(nsFilteredContentIterator * const 0x00fbafd8)
line 158
nsTextServicesDocument::FirstTextNode(nsIContentIterator * 0x03067f20,
nsTextServicesDocument::TSDIteratorStatus * 0x03067e64) line 4123
nsTextServicesDocument::FirstBlock(nsTextServicesDocument * const 0x03067e50)
line 608 + 10 bytes
nsEditorSpellCheck::InitSpellChecker(nsEditorSpellCheck * const 0x03067d90,
nsIEditor * 0x03b63530, int 0) line 93
mozInlineSpellChecker::SetEnableRealTimeSpell(mozInlineSpellChecker * const
0x03c56800, int 50757008) line 174
XPTC_InvokeByIndex(nsISupports * 0x03c56800, unsigned int 7, unsigned int 1,
nsXPTCVariant * 0x0012d684) line 102
... the rest is the usual stack from opening a window and running JS stuff.
(Assignee)

Comment 9

14 years ago
I've done a bit of debugging and I can see that when a new compose window is
created the body element has at least one child that the content iterator range
is selecting but when it is recycled the range is collapsed.
(Assignee)

Comment 10

14 years ago
OK, so it turns out that a new editor body contains a <br> ...
> Interestingly I run my linux build with the recycled compose window pref

I never managed to reproduce this bug in a debug build.  I tested backing out
bug 294251 by editing my chrome in a nightly...

I bet the reason we're not blinking is that nothing is on the line when we
recycle.  This is a known caret issue; that's why editor sticks <br> all over.
(Assignee)

Comment 12

14 years ago
OK, I have some minimal steps to reproduce that also work on Thunderbird.
1. Start Suite/TB
2. Turn off spell as you type in preferences
3. Write a new message
4. Using the options menu, turn spell as you type on and off again
5. Close the window
6. Write a new message
The recycled body will not have a caret.
(Assignee)

Comment 13

14 years ago
(In reply to comment #11)
>>Interestingly I run my linux build with the recycled compose window pref
>I never managed to reproduce this bug in a debug build.  I tested backing out
>bug 294251 by editing my chrome in a nightly...
Oh, it shows up fine in my MSVC debug build.

>I bet the reason we're not blinking is that nothing is on the line when we
>recycle.  This is a known caret issue; that's why editor sticks <br> all over.
The document body appears to contain a <br> either way; what I can't figure is
why it doesn't work the second time.
Changing summary FROM: Caret disappears after tabbing to Composer Body

TO: Caret disappears in cached compose window body
Summary: Caret disappears after tabbing to Composer Body → Caret disappears in cached compose window body
(Assignee)

Comment 15

14 years ago
OK, I can now reproduce this without step 4...
(Assignee)

Comment 16

14 years ago
... and I can reproduce in 2005051905 ...
(Assignee)

Comment 17

14 years ago
In fact my regression range is 2005-05-12-06 to 2005-05-13-06.
But nothing in that range strike me as being plausible :-(
(Assignee)

Comment 18

14 years ago
I double-checked and my regression range is actually 2005-05-13-06 to
2005-05-14-06 - just when Spell As You Type originally landed...
(Assignee)

Comment 19

14 years ago
(In reply to comment #11)
>I bet the reason we're not blinking is that nothing is on the line when we
>recycle.  This is a known caret issue; that's why editor sticks <br> all over.
You're absolutely right as usual.
When the inline spell checker is disabled, it returns error codes from its post
edit hook. This prevents various post-edit actions from occuring. In particular
the very next call after the error check is to CreateBogusNodeIfNeeded...
Component: MailNews: Composition → Spelling checker
(Assignee)

Comment 20

14 years ago
Created attachment 185414 [details] [diff] [review]
Proposed patch

I'm surprised that turning off inline spellcheck doesn't make more stuff fail.
Assignee: nobody → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Attachment #185414 - Flags: superreview?(mscott)
Attachment #185414 - Flags: review?(mscott)

Updated

14 years ago
Attachment #185414 - Flags: superreview?(mscott)
Attachment #185414 - Flags: superreview+
Attachment #185414 - Flags: review?(mscott)
Attachment #185414 - Flags: review+
(Reporter)

Comment 21

14 years ago
Comment on attachment 185414 [details] [diff] [review]
Proposed patch

regression
Attachment #185414 - Flags: approval1.8b3?

Updated

14 years ago
Attachment #185414 - Flags: approval1.8b3? → approval1.8b3+
(Assignee)

Comment 22

14 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
I've been using daily builds and haven't seen this crop up since the fix landed.

Verified FIXED using build 2005-06-11-05 on Windows XP Seamonkey trunk.  (I
double-checked the steps in comment 0, comment 4, and comment 12.)
Status: RESOLVED → VERIFIED

Updated

14 years ago
Flags: blocking-aviary1.1?
You need to log in before you can comment on or make changes to this bug.