299676 - Crash with this editor testcase using an iframe [@ 0x0a013520 nsCutCommand::IsCommandEnabled]

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase, follow the steps also mentioned in the testcase:
1. - Click on button document.designMode='on'<br>
2. Click on link inside iframe of Google, a blank window appears which is a bug
in itself<br>
3. Click on 'Reload'<br>
4. click on 'Back' -> Crash

This is not a regression.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

Putting [@ 0x0a013520 nsCutCommand::IsCommandEnabled] in the Summary for now,
but note that a variety of steps will crash in different places after the reload
(such as another reload, an Edit | Select All command, etc.)

0x0a013520
nsCutCommand::IsCommandEnabled 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/editor/libeditor/base/nsEditorCommands.cpp,
line 212]
nsControllerCommandTable::IsCommandEnabled 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/embedding/components/commandhandler/src/nsControllerCommandTable.cpp,
line 138]
nsBaseCommandController::IsCommandEnabled 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/embedding/components/commandhandler/src/nsBaseCommandController.cpp,
line 117]
XPTC_InvokeByIndex 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2119]
XPC_WN_CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1348]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1178]
js_Interpret 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 3469]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1198]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1275]
JS_CallFunctionValue 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line
3920]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1415]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/events/nsJSEventListener.cpp,
line 184]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1580]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1681]
nsXULElement::HandleDOMEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2201]
nsXULCommandDispatcher::UpdateCommands 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/document/src/nsXULCommandDispatcher.cpp,
line 386]
nsGlobalWindow::UpdateCommands 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 3751]
nsDocViewerSelectionListener::NotifySelectionChanged 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsDocumentViewer.cpp,
line 3175]
nsTypedSelection::NotifySelectionListeners 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsSelection.cpp,
line 7278]
nsSelection::NotifySelectionListeners 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsSelection.cpp,
line 3004]
nsHTMLEditor::BeginningOfDocument 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/editor/libeditor/html/nsHTMLEditor.cpp,
line 521]
nsEditor::Init 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/editor/libeditor/base/nsEditor.cpp,
line 321]
nsPlaintextEditor::Init 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/editor/libeditor/text/nsPlaintextEditor.cpp,
line 160]
nsHTMLEditor::Init 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/editor/libeditor/html/nsHTMLEditor.cpp,
line 283]
nsEditingSession::SetupEditorOnWindow 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/editor/composer/src/nsEditingSession.cpp,
line 456]
nsEditingSession::EndDocumentLoad 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/editor/composer/src/nsEditingSession.cpp,
line 1113]
nsEditingSession::OnStateChange 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/editor/composer/src/nsEditingSession.cpp,
line 818]
nsDocLoader::FireOnStateChange 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/uriloader/base/nsDocLoader.cpp,
line 1199]
nsDocLoader::doStopDocumentLoad 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/uriloader/base/nsDocLoader.cpp,
line 833]
nsDocLoader::DocLoaderIsEmpty 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/uriloader/base/nsDocLoader.cpp,
line 733]
nsDocLoader::OnStopRequest 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/uriloader/base/nsDocLoader.cpp,
line 654]
nsLoadGroup::RemoveRequest 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/netwerk/base/src/nsLoadGroup.cpp,
line 732]
PresShell::RemoveDummyLayoutRequest 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 7085]
DummyLayoutRequestEvent::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsPresShell.cpp,
line 6985]
SHDOCVW.dll + 0x150c24 (0x778b0c24)
0x006e006f

Comment 3

[[e-mail]]

OK, so the deal is this. Midas sets up a parent content listener on its docshell
to stop it being targetted by link loads. When you click the link on the inner
frame the content listener rejects the load so the doc loader tries to find a
registered content listener. As it happens, the browser chrome registers a
content listener which re-accepts the load into the very same doc shell...

Comment 4

[[e-mail]]

Attachment: Proposed patch

What this does is stop links from e.g. the editor sidebar from opening in a
midas window, but it also applies to links in inner iframes.

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

sr=bzbarsky, but could we factor out the "get the content listener" code that
we have in 3 or 4 places in each of those impls into a separate function,
perhaps?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

So when you click on a link in the testcase, it opens in a new window? Is that
really what we want here? I think another approach would be to install another
content listener for the iframe when we install midas' content listener, so that
when you click on a link in the <iframe>, we check that one, which returns the
iframe's docshell, and things should be happy.

Also, when I apply this patch and click on a link, I get a new window, but it
says: The file /res/[xpconnect wrapped nsIURI @ 0x896e278 (native @ 0x87af3bc)]
cannot be found. Please check the location and try again

Comment 7

[Martijn Wargers (dead)]

In current trunk build, the testcase is crashing even earlier.
Just clicking on the document.designMode='on' button and then on a link in the iframe triggers the crash.

Comment 8

[Martijn Wargers (dead)]

I now crash directly when clicking on the link in the iframe (after setting designMode to on), Talkback ID TB15002251Q.

Comment 9

[Hermann Schwab]

Attachment: Talkback Record from testcase

fresh Talkback Record TB17006817Q from testcase is crashing as described in comment #8, TB15002251Q from comment #8  is gone.

TB17006817Q is differing only in the top stack line from TB16990609W Bug 331981

Comment 10

[chris hofmann]

does the patch need rework for blake's input in Comment #6 , or do we just need mconnors review to make progress on getting this checked in.

Comment 11

[Mike Connor [:mconnor]]

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

Hopefully I'm not misunderstanding the issue here, but I think that in most cases, you don't want links inside of a midas instance to open in the midas instance, so this should be fine...

Comment 12

[[e-mail]]

I'm not hitting any content listener any more...

Comment 13

[Martijn Wargers (dead)]

Fixed by bug 334515.

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using 04-27-16 build of SeaMonkey trunk on Windows XP.