Closed
Bug 296265
Opened 20 years ago
Closed 19 years ago
Caret disappears in cached compose window body
Categories
(Core :: Spelling checker, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: mcsmurf, Assigned: neil)
References
Details
(Keywords: regression)
Attachments
(1 file)
828 bytes,
patch
|
mscott
:
review+
mscott
:
superreview+
asa
:
approval1.8b3+
|
Details | Diff | Splinter Review |
To reproduce:
1. Open MailNews Composer
2. Paste some text into subject
3. Press tab key
4. Observe that caret is invisible
Reporter | ||
Comment 1•20 years ago
|
||
This regressed between 2005-05-31-06 and 2005-06-01-05. Bonsai link:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-05-31+05%3A00%3A00&maxdate=2005-06-01+06%3A00%3A00&cvsroot=%2Fcvsroot
I am seeing this same bug with 2005052706, seamonkey win2k. Are you sure the
regression window is correct?
Comment 3•20 years ago
|
||
Is this related?
Error: Unknown namespace prefix 'html'. Ruleset ignored due to bad selector.
Source File: chrome://messenger/skin/messageBody.css
Line: 54
Comment 4•20 years ago
|
||
(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.
Comment 5•20 years ago
|
||
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?
Comment 6•20 years ago
|
||
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.
Comment 7•20 years ago
|
||
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
Updated•20 years ago
|
Flags: blocking-aviary1.1?
Assignee | ||
Comment 8•20 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•20 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•20 years ago
|
||
OK, so it turns out that a new editor body contains a <br> ...
Comment 11•20 years ago
|
||
> 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•20 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•20 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•20 years ago
|
||
OK, I can now reproduce this without step 4...
Assignee | ||
Comment 16•20 years ago
|
||
... and I can reproduce in 2005051905 ...
Assignee | ||
Comment 17•20 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•19 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•19 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•19 years ago
|
||
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•19 years ago
|
Attachment #185414 -
Flags: superreview?(mscott)
Attachment #185414 -
Flags: superreview+
Attachment #185414 -
Flags: review?(mscott)
Attachment #185414 -
Flags: review+
Reporter | ||
Comment 21•19 years ago
|
||
Comment on attachment 185414 [details] [diff] [review]
Proposed patch
regression
Attachment #185414 -
Flags: approval1.8b3?
Updated•19 years ago
|
Attachment #185414 -
Flags: approval1.8b3? → approval1.8b3+
Assignee | ||
Comment 22•19 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 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•19 years ago
|
Flags: blocking-aviary1.1?
You need to log in
before you can comment on or make changes to this bug.
Description
•