4584 - Clicking on alternate image text causes crash

Status: VERIFIED FIXED

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

Description

[kinmoz]

The Win32 04/06/99 build of the editor testbed fails to render the inserted
image. Instead, it displays the alternate text. If you try to click on the
alternate text, you crash.

This might be related to bug #4383.

Steps to reproduce:

1. run apprunner -editor

2. Place the cursor in any Non-preformatted text block.

3. Select Insert->Image from the menu.

Notice how the string "[pen splat]" appears instead of the image.

If you try to place the cursor in the string "[pen splat]", you crash with the
following stack trace:

nsGenericDOMDataNode::HandleDOMEvent(nsIPresContext & {...}, nsEvent *
0x0012fb30, nsIDOMEvent * * 0x0012f874, unsigned int 1, nsEventStatus &
nsEventStatus_eConsumeDoDefault) line 739 + 33 bytes
nsTextNode::HandleDOMEvent(nsTextNode * const 0x014f7b1c, nsIPresContext &
{...}, nsEvent * 0x0012fb30, nsIDOMEvent * * 0x00000000, unsigned int 1,
nsEventStatus & nsEventStatus_eConsumeDoDefault) line 189
PresShell::HandleEvent(PresShell * const 0x014c7404, nsIView * 0x014caa00,
nsGUIEvent * 0x0012fb30, nsEventStatus & nsEventStatus_eConsumeDoDefault) line
2062 + 34 bytes
nsView::HandleEvent(nsView * const 0x014caa00, nsGUIEvent * 0x0012fb30, unsigned
int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 825
nsView::HandleEvent(nsView * const 0x014c99d0, nsGUIEvent * 0x0012fb30, unsigned
int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 808
nsView::HandleEvent(nsView * const 0x014c98f0, nsGUIEvent * 0x0012fb30, unsigned
int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 808
nsScrollingView::HandleEvent(nsScrollingView * const 0x014c98f0, nsGUIEvent *
0x0012fb30, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault)
line 840
nsView::HandleEvent(nsView * const 0x014c33c0, nsGUIEvent * 0x0012fb30, unsigned
int 28, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 808
nsViewManager::DispatchEvent(nsViewManager * const 0x014be750, nsGUIEvent *
0x0012fb30, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 1719
HandleEvent(nsGUIEvent * 0x0012fb30) line 64
nsWindow::DispatchEvent(nsWindow * const 0x014ca130, nsGUIEvent * 0x0012fb30,
nsEventStatus & nsEventStatus_eIgnore) line 416 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012fb30) line 437
nsWindow::DispatchMouseEvent(unsigned int 300, nsPoint * 0x00000000) line 2474 +
15 bytes
ChildWindow::DispatchMouseEvent(unsigned int 300, nsPoint * 0x00000000) line
2623
nsWindow::ProcessMessage(unsigned int 512, unsigned int 1, long 32833748, long *
0x0012fe60) line 1821 + 24 bytes
nsWindow::WindowProc(void * 0xeb3b057e, unsigned int 512, unsigned int 1, long
32833748) line 480 + 27 bytes
USER32! 77e71250()
01f500d4()


We crash because the this->mParent node has already been deleted.

Comment 1

[kinmoz]

I should also mention that the Debug->Output HTML menu item reports that the
image is indeed in the content tree.

Comment 2

[kinmoz]

I figured out why we are crashing.

When layout can't find/load the image to display, it displays the alternate
text. To do this, it replaces the image frame, in the frame list, with a text
frame that points to a dummy content TextNode that contains the alternate text.
This dummy node is not added to the DOM or content tree, because it wants to
keep the original image content and DOM node. Because this dummy node is not
added to any tree, it's mParent pointer is always NULL.

When you try to click on the alternate text, nsRangeList::TakeFocus() figures
out what frame was clicked on, in this case it is the text frame that points to
the dummy alternate text node. TakeFocus() then gets the content node associated
with the frame, then gets a handle to it's DOM node equivalent. It then calls
domNode->GetParentNode() which ends up calling
nsGenericDOMDataNode::GetParentNode().

If the domNode's mParent pointer is NULL, nsGenericDOMDataNode::GetParentNode()
creates and sets a document fragment as the domNode's parent. This new document
fragment, with a RefCount == 1, is then returned to TakeFocus() and stored in a
smart pointer. When this smart pointer goes out of scope, it calls release on
the parent node, causing it to be deleted. Note that nodes never addref pointers
to their parents.

The workaround for this would be for TakeFocus() to call
nsIContent::GetParent(), instead of nsIDOMNode::GetParentNode(), to check if
the frame/content is really in the tree. In the case of the alt text, it would
return null, indicating that it is not in the DOM/Content tree, so there would
be nothing to do.

I spoke to [e-mail], who wrote TakeFocus(), who implemented this
workaround, and it seems to stop the crashing.

I'm changing the summary line from "InsertImage displays alternate text and
crashes" to say "Clicking on alternate image text causes crash", and will assign
it to mjudge.

I've opened bug #4661 to track the fact that the alt text is being displayed
instead of the image on Win32.

Comment 3

[sujay]

verified in 4/29 build...I simply click on the "[pen splat]" text
and don't see any crash...so I presume this is working now...

Comment 4

[Hixie (not reading bugmail)]

This bug is one of many related to alternate text of images. All these bugs
have been marked. To find related bugs, search the description field for the
string "[ALT]".