Bug 43354 - Potential Null Crasher in ReplaceSelectionCommand
Summary: Potential Null Crasher in ReplaceSelectionCommand
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-02 08:46 PDT by Eric Seidel (no email)
Modified: 2012-05-25 00:06 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-08-02 08:46:58 PDT
http://trac.webkit.org/browser/trunk/WebCore/editing/ReplaceSelectionCommand.cpp#L935

935	    if ((isListElement(refNode.get()) || (isStyleSpan(refNode.get()) && isListElement(refNode->firstChild())))
936	        && blockStart->renderer()->isListItem())
937	        refNode = insertAsListItems(refNode, blockStart, insertionPos);

I suspect that unguarded renderer()-> call could crash.  Nodes don't have to have renderers.
Comment 1 Eric Seidel (no email) 2010-08-02 08:47:12 PDT
I'm about to change that line of code a little.
Comment 2 Ryosuke Niwa 2010-08-02 09:10:59 PDT
+tony since svn annotate indicates he has edited that part of code in two changesets 54413 and 54931.
Comment 3 Tony Chang 2010-08-02 09:35:54 PDT
We should probably just use isListItem() from htmlediting.h, although I'm surprised that enclosingBlock() can return a node w/o a renderer.  Maybe <html> is a block w/o a renderer?
Comment 4 Ryosuke Niwa 2012-05-25 00:06:40 PDT
Node* blockStart = enclosingBlock(insertionPos.deprecatedNode());

But 

bool isBlock(const Node* node)
{
    return node && node->renderer() && !node->renderer()->isInline() && !node->renderer()->isRubyText();
}

Element* enclosingBlock(Node* node, EditingBoundaryCrossingRule rule)
{
    Node* enclosingNode = enclosingNodeOfType(firstPositionInOrBeforeNode(node), isBlock, rule);
    return enclosingNode && enclosingNode->isElementNode() ? toElement(enclosingNode) : 0;
}

So this can never cause a crash.