42145 - Crash on sonydirect.com

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Phil Peterson]

Using 06-09-09 commercial build on NT

1. Go to http://www.sonydirect.com/sneakpreview
2. Click on SR series notebooks
3. Popup window appears 
4. Click in content area of popup window
5. Boom

I think that step (3) may be a bug, since in the 4.x product, we don't show a 
popup window, but rather the image in the window displays in the page when you 
roll the mouse over the link.

I filed a Talkback incident 12158267 on this, and the stack trace seems to point 
to the editor clipboard command-enabling code, so I'll start with the editor.

Comment 1

[Phil Peterson]

Attachment: Stack trace

Comment 2

[rubydoo123]

assigning to Simon

Comment 3

[rubydoo123]

asked Sujay to test on other platoforms and to get us a reduced test case.

Comment 4

[sujay]

I cannot reproduce this at all...I tried Win 98, Mac, Linux using latest
builds..therefore I can't come up with a reduced test case either...

Comment 5

[Phil Peterson]

Worksforme in today's build too. Now when the popup window opens, the cursor
does not turn into the link-clicking-finger cursor, so I suspect some content-
side change.

Comment 6

[Simon Fraser [no longer active]]

Reopening based on akkana's comment:

Line 102 of nsEditorCommands.cpp is nsCutCommand::IsCommandEnabled 
dereferencing its string argument. So we could stop the crash by 
checking the string argument in each of the nsEditorCommands (if we 
change nsCutCommand, we should presumably change all the rest of the 
commands as well) but that doesn't explain how a null pointer could 
have been passed through xpconnect, which appears to be what happened 
here. 

Comment 7

[Simon Fraser [no longer active]]

This still crashes mac.

Comment 8

[Simon Fraser [no longer active]]

I added assertions in the command code to catch cases where they were being 
called with null arguments, and never hit them (but still crashed). I think this 
bug is memory corruption somewhere, but I've not been able to track it down.

Comment 9

[Simon Fraser [no longer active]]

Maybe someone with purify can help?

Comment 10

[Akkana Peck]

I don't crash when following these steps.  But the content area of the dialog
that comes up isn't a link or anything else clickable (in fact, I can't even
select in it).

Comment 11

[Simon Fraser [no longer active]]

Well, I see at least one thing bad here. nsGfxTextControlFrame2::Destroy() needs 
to clear out the editor in its controller, thusly:

Index: nsGfxTextControlFrame2.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsGfxTextControlFrame2.cpp,v
retrieving revision 1.35
diff -w -c -1 -r1.35 nsGfxTextControlFrame2.cpp
*** nsGfxTextControlFrame2.cpp	2000/06/14 03:01:31	1.35
--- nsGfxTextControlFrame2.cpp	2000/06/14 18:55:20
***************
*** 723,724 ****
--- 723,755 ----
  {
+   // Clean up the controller
+   nsCOMPtr<nsIDOMNSHTMLTextAreaElement> textAreaElement = 
do_QueryInterface(mContent);
+   nsCOMPtr<nsIDOMNSHTMLInputElement>    inputElement = 
do_QueryInterface(mContent);
+   nsCOMPtr<nsIControllers> controllers;
+   nsresult rv;
+   if (textAreaElement)
+     rv = textAreaElement->GetControllers(getter_AddRefs(controllers));
+   else if (inputElement)
+     rv = inputElement->GetControllers(getter_AddRefs(controllers));
+   else
+     rv = NS_ERROR_FAILURE;
+   if (NS_SUCCEEDED(rv))
+   {
+     PRUint32 numControllers;
+     rv = controllers->GetControllerCount(&numControllers);
+     NS_ASSERTION((NS_SUCCEEDED(rv)), "bad result in gfx text control 
destructor");
+     for (PRUint32 i = 0; i < numControllers; i ++)
+     {
+       nsCOMPtr<nsIController> controller;
+       rv = controllers->GetControllerAt(i, getter_AddRefs(controller));
+       if (NS_SUCCEEDED(rv) && controller)
+       {
+         nsCOMPtr<nsIEditorController> editController = 
do_QueryInterface(controller);
+         if (editController)
+         {
+           editController->SetCommandRefCon(nsnull);
+         }
+       }
+     }
+   }
+ 
    mSelCon = 0;

Comment 12

[Simon Fraser [no longer active]]

That patch fixes the crash on Windows. However, on Mac, we then crash down in 
nsHTMLInputElement::HandleDOMEvent(), when handling an update event for the 
window, because the frame returned from nsGenericHTMLElement::GetPrimaryFrame() 
is null.

saari: nsEventStateManager is calling HandleDOMEvent on gLastFocusedContent 
(which is the input element), even though that input field has been hidden, so it 
no longer has any frames. It seems that something needs to tell 
nsEventStateManager to nuke its gLastFocusedContent pointer when content is 
dynamically hidden, as in this case. Over to you...

BTW, I'll check in an assertion in nsHTMLInputElement::HandleDOMEvent() that 
you'll hit in this case, instead of crashing badly.

Comment 13

[Simon Fraser [no longer active]]

Ooops, it was actually crashing in code that I added, so there is no crash now. 
However, It still seems wrong that we're calling HandleDOMEvent on an element 
that is hidden.

Comment 14

[Simon Fraser [no longer active]]

Fix checked in.

Comment 15

[Phil Peterson]

Hmm. Sort of fixed. Now we open the popup window rather than crashing, but the 
Sneak Preview page under the popup window blanks out. Is this a different bug?

Comment 16

[Simon Fraser [no longer active]]

I'm pretty sure that would be different; my changes will not affect URL loading.

Comment 17

[sujay]

verified in 6/29 build.