204204 - Crash in editor when shift-tabbing out of a modified htmlarea [@ nsHTMLCSSUtils::IsCSSEditableProperty]

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Ere Maijala (slow)]

I'm getting constant crashes when shift tabbing out of a modified htmlarea 3.0
alpha. Steps to reproduce: 
1. Load the test page from http://atp.fi/~ere/crash/crashtest.html
2. verify that the editor has focus
3. shift-tab

It seems that it crashes at line 344 of nsHTMLCssUtils.cpp, calling 
content->GetTag(*getter_AddRefs(tagName)); 

Call stack:

nsHTMLCSSUtils::IsCSSEditableProperty(nsIDOMNode * 0x04767eac, nsIAtom *
0x00000000, const nsAString * 0x0012973c) line 344 + 44 bytes

nsHTMLEditRules::GetAlignment(nsHTMLEditRules * const 0x047b0f88, int *
0x001298a0, short * 0x00129808) line 776 + 44 bytes

nsHTMLEditor::GetAlignment(nsHTMLEditor * const 0x049f4930, int * 0x001298a0,
short * 0x00129808) line 2658 + 31 bytes

nsAlignCommand::GetCurrentState(nsIEditor * 0x049f4848, nsICommandParams *
0x045a0588) line 1050 + 49 bytes

nsMultiStateCommand::GetCommandStateParams(nsMultiStateCommand * const
0x049bcb98, const char * 0x00129b10, nsICommandParams * 0x045a0588, nsISupports
* 0x049f4848) line 686 + 24 bytes

nsControllerCommandTable::GetCommandState(nsControllerCommandTable * const
0x042931d0, const char * 0x00129b10, nsICommandParams * 0x045a0588, nsISupports
* 0x049f4848) line 226 + 35 bytes

nsBaseCommandController::GetCommandStateWithParams(nsBaseCommandController *
const 0x04a2e5e4, const char * 0x00129b10, nsICommandParams * 0x045a0588) line 149

nsCommandManager::GetCommandState(nsCommandManager * const 0x04766158, const
char * 0x00129b10, nsIDOMWindow * 0x04a0d5e4, nsICommandParams * 0x045a0588)
line 234 + 43 bytes

nsHTMLDocument::QueryCommandState(nsHTMLDocument * const 0x04768058, const
nsAString & {...}, int * 0x00129cf0) line 4488 + 63 bytes

XPTC_InvokeByIndex(nsISupports * 0x04768058, unsigned int 0x00000022, unsigned
int 0x00000002, nsXPTCVariant * 0x00129ce0) line 102

XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_METHOD) line 2023 + 42 bytes

XPC_WN_CallMethod(JSContext * 0x037dff78, JSObject * 0x04924bf8, unsigned int
0x00000001, long * 0x043af0c0, long * 0x00129fc0) line 1284 + 14 bytes
js_Invoke(JSContext * 0x037dff78, unsigned int 0x00000001, unsigned int
0x00000000) line 843 + 23 bytes

js_Interpret(JSContext * 0x037dff78, long * 0x0012ae80) line 2834 + 15 bytes
js_Invoke(JSContext * 0x037dff78, unsigned int 0x00000000, unsigned int
0x00000000) line 860 + 13 bytes

js_Interpret(JSContext * 0x037dff78, long * 0x0012bcf4) line 2834 + 15 bytes

js_Invoke(JSContext * 0x037dff78, unsigned int 0x00000001, unsigned int
0x00000002) line 860 + 13 bytes

js_InternalInvoke(JSContext * 0x037dff78, JSObject * 0x02e07968, long
0x03e712b8, unsigned int 0x00000000, unsigned int 0x00000001, long * 0x04a0c310,
long * 0x0012be20) line 935 + 20 bytes

JS_CallFunctionValue(JSContext * 0x037dff78, JSObject * 0x02e07968, long
0x03e712b8, unsigned int 0x00000001, long * 0x04a0c310, long * 0x0012be20) line
3527 + 31 bytes

nsJSContext::CallEventHandler(nsJSContext * const 0x03e69df0, void * 0x02e07968,
void * 0x03e712b8, unsigned int 0x00000001, void * 0x04a0c310, int * 0x0012bea8,
int 0x00000000) line 1079 + 33 bytes

GlobalWindowImpl::RunTimeout(nsTimeoutImpl * 0x047b80d0) line 4906 + 84 bytes

GlobalWindowImpl::TimerCallback(nsITimer * 0x047bc938, void * 0x047b80d0) line 5266

nsTimerImpl::Fire() line 382 + 17 bytes

nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x0107ffa0) line 616

Comment 1

[Kathleen :Brade]

so far ere has found out this:
content is null from QI; perhaps because the node is a document?
perhaps GetBlockNodeParent should stop at the root of the editor and it isn't?

I see this crash on macho.

Comment 2

[kinmoz]

The problem seems to be that the event state manager code is setting the
selection so that it is at (html, 0), and the editor never expects the selection
to go above it's root node which is the body. Cc'ing aaronl because cvs blame
shows he added the code.

Here is the stack to where it adds the range:


nsTypedSelection::AddRange(nsTypedSelection * const 0x0375bab8, nsIDOMRange *
0x02e0d620) line 5971
nsEventStateManager::MoveCaretToFocus(nsEventStateManager * const 0x0340dab0)
line 4977
nsEventStateManager::ShiftFocusInternal(int 0, nsIContent * 0x00000000) line 3254
nsEventStateManager::ShiftFocus(nsEventStateManager * const 0x0340dab0, int 0,
nsIContent * 0x00000000) line 3032
nsEventStateManager::PostHandleEvent(nsEventStateManager * const 0x0340dab0,
nsIPresContext * 0x034401e8, nsEvent * 0x0012f77c, nsIFrame * 0x033c51fc,
nsEventStatus * 0x0012f5a8, nsIView * 0x033b48d0) line 2048
PresShell::HandleEventInternal(nsEvent * 0x0012f77c, nsIView * 0x033b48d0,
unsigned int 1, nsEventStatus * 0x0012f5a8) line 6414 + 43 bytes
PresShell::HandleEvent(PresShell * const 0x033f4384, nsIView * 0x033b48d0,
nsGUIEvent * 0x0012f77c, nsEventStatus * 0x0012f5a8, int 1, int & 1) line 6295 +
25 bytes
nsViewManager::HandleEvent(nsView * 0x033b48d0, nsGUIEvent * 0x0012f77c, int 0)
line 2246
nsView::HandleEvent(nsViewManager * 0x033b46a0, nsGUIEvent * 0x0012f77c, int 0)
line 308
nsViewManager::DispatchEvent(nsViewManager * const 0x033b46a0, nsGUIEvent *
0x0012f77c, nsEventStatus * 0x0012f6ec) line 2022 + 23 bytes
HandleEvent(nsGUIEvent * 0x0012f77c) line 82
nsWindow::DispatchEvent(nsWindow * const 0x033e4c9c, nsGUIEvent * 0x0012f77c,
nsEventStatus & nsEventStatus_eIgnore) line 1053 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f77c) line 1074
nsWindow::DispatchKeyEvent(unsigned int 131, unsigned short 0, unsigned int 9,
long 0) line 2911 + 15 bytes
nsWindow::OnChar(unsigned int 9, unsigned int 9, unsigned char 0) line 3098
nsWindow::ProcessMessage(unsigned int 258, unsigned int 9, long 983041, long *
0x0012fc30) line 3810 + 38 bytes
nsWindow::WindowProc(HWND__ * 0x002e04aa, unsigned int 258, unsigned int 9, long
983041) line 1336 + 27 bytes
USER32! 77e11d0a()
USER32! 77e11bc8()
USER32! 77e11cef()
nsAppShellService::Run(nsAppShellService * const 0x0146a2a8) line 479
main1(int 2, char * * 0x00262698, nsISupports * 0x00e34d00) line 1268 + 32 bytes
main(int 2, char * * 0x00262698) line 1647 + 37 bytes
mainCRTStartup() line 338 + 17 bytes

Comment 3

[sairuh (rarely reading bugmail)]

kin, out of curiosity, when was aaronl's change made?

Comment 4

[Greg K.]

Attachment: Crash report generated by FizzillaMach/2003-05-01-08-trunk showing crash at nsHTMLCSSUtils::IsCSSEditableProperty

Comment 5

[Aaron Leventhal]

Attachment: Move focus to caret only when browsing, not when editing

IMHO, this seems like the safest thing to do. We really don't want the
MoveFocusToCaret() to do anything in the editor at all. It is only to help us
keep a single point of regard when navigating in the browser. In the editor,
the focus should always be on the document itself, not items within it.

Comment 6

[Aaron Leventhal]

Comment on attachment 122453 [details] [diff] [review]
Move focus to caret only when browsing, not when editing

Either one of you can give me sr=, but I need an one r= as well. :-)

Comment 7

[Kathleen :Brade]

Comment on attachment 122453 [details] [diff] [review]
Move focus to caret only when browsing, not when editing

Changing bryner to sr since Kin is on vacation for two weeks.

This fix seems fine to me but I still think there is a problem in the editor.

Comment 8

[Brian Ryner (not reading)]

Comment on attachment 122453 [details] [diff] [review]
Move focus to caret only when browsing, not when editing

sr=bryner

Comment 9

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

Comment on attachment 122453 [details] [diff] [review]
Move focus to caret only when browsing, not when editing

a=sspitzer

Comment 10

[Kathleen :Brade]

-->aaronl since it's his patch

aaronl: did you already land this fix?

Comment 11

[Aaron Leventhal]

checked in

REOPEN if you want to do more work on the editor side.

Comment 12

[sairuh (rarely reading bugmail)]

vrfy'd fixed using the url test case, with 2003.05.14 comm builds.