http://code.google.com/p/chromium/issues/detail?id=90306 1. Open the URL 2. Focus the search field 3. Type the Del key (Backspace in Windows) 4. Crash! This is reproducible with Safari 5.0.5 - Today's WebKit nightly. In the nightly, this is a null pointer dereference.
Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000 0x00000001022f7f1c in WebCore::RenderTextControl::computeLogicalHeight (this=0x12b3b23a8) at WebKit/Source/WebCore/rendering/RenderTextControl.cpp:288 288 innerTextRenderBox->marginTop() + innerTextRenderBox->marginBottom()); (gdb) backtrace #0 WebCore::RenderTextControl::computeLogicalHeight (this=0x12b3b23a8) at WebKit/Source/WebCore/rendering/RenderTextControl.cpp:288 #1 WebCore::RenderTextControlSingleLine::layout (this=0x12b3b23a8) at WebKit/Source/WebCore/rendering/RenderTextControlSingleLine.cpp:223 #2 WebCore::FrameView::layout (this=0x12b3ab190, allowSubtree=true) at WebKit/Source/WebCore/page/FrameView.cpp:1016 #3 WebCore::Document::updateLayout (this=0x12c07ae00) at WebKit/Source/WebCore/dom/Document.cpp:1620 #4 WebCore::Document::updateLayoutIgnorePendingStylesheets (this=0x12c07ae00) at WebKit/Source/WebCore/dom/Document.cpp:1651 #5 WebCore::EditCommand::updateLayout (this=0x12b21b490) at WebKit/Source/WebCore/editing/EditCommand.cpp:208 #6 WebCore::DeleteSelectionCommand::fixupWhitespace (this=0x12b21b490) at WebKit/Source/WebCore/editing/DeleteSelectionCommand.cpp:558 #7 WebCore::DeleteSelectionCommand::doApply (this=0x12b21b490) at WebKit/Source/WebCore/editing/DeleteSelectionCommand.cpp:832 #8 WebCore::EditCommand::apply (this=0x12b21b490) at WebKit/Source/WebCore/editing/EditCommand.cpp:92 #9 WebCore::CompositeEditCommand::applyCommandToComposite (this=0x12b21b010, cmd=@0x7fff5fbfe0e0) at WebKit/Source/WebCore/editing/CompositeEditCommand.cpp:102 #10 WebCore::CompositeEditCommand::deleteSelection (this=0x12b21b010, selection=@0x7fff5fbfe2e0, smartDelete=false, mergeBlocksAfterDelete=true, replace=false, expandForSpecialElements=true) at WebKit/Source/WebCore/editing/CompositeEditCommand.cpp:409 #11 WebCore::TypingCommand::deleteKeyPressed (this=0x12b21b010, granularity=WebCore::CharacterGranularity, killRing=false) at WebKit/Source/WebCore/editing/TypingCommand.cpp:548 #12 WebCore::TypingCommand::doApply (this=0x12b21b010) at WebKit/Source/WebCore/editing/TypingCommand.cpp:292 #13 WebCore::EditCommand::apply (this=0x12b21b010) at WebKit/Source/WebCore/editing/EditCommand.cpp:92 #14 WebCore::TypingCommand::deleteKeyPressed (document=0x12c07ae00, options=0, granularity=WebCore::CharacterGranularity) at WebKit/Source/WebCore/editing/TypingCommand.cpp:117 #15 WebCore::Editor::deleteWithDirection (this=0x109015a38, direction=WebCore::DirectionBackward, granularity=WebCore::CharacterGranularity, killRing=false, isTypingAction=true) at WebKit/Source/WebCore/editing/Editor.cpp:315 #16 WebCore::executeDeleteBackward (frame=0x109015400) at WebKit/Source/WebCore/editing/EditorCommand.cpp:330 #17 WebCore::Editor::Command::execute (this=0x7fff5fbfe700, parameter=@0x7fff5fbfe690, triggeringEvent=0x12b2195c0) at WebKit/Source/WebCore/editing/EditorCommand.cpp:1648 #18 WebCore::Editor::Command::execute (this=0x7fff5fbfe700, triggeringEvent=0x12b2195c0) at WebKit/Source/WebCore/editing/EditorCommand.cpp:1653 #19 -[WebHTMLView(WebNSTextInputSupport) doCommandBySelector:] (self=0x12b3a7470, _cmd=0x7fff8295951c, selector=0x7fff8298f4cf) at WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:5805 #20 -[WebHTMLView(WebInternal) _executeSavedKeypressCommands] (self=0x12b3a7470, _cmd=0x101218398) at WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:5274 #21 -[WebHTMLView(WebInternal) _interpretKeyEvent:savingCommands:] (self=0x12b3a7470, _cmd=0x1011ff8c2, event=0x12b2195c0, savingCommands=0 '\0') at WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:5333 #22 WebEditorClient::handleKeyboardEvent (this=0x1089258a0, event=0x12b2195c0) at WebKit/Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm:593 #23 WebCore::Editor::handleKeyboardEvent (this=0x109015a38, event=0x12b2195c0) at WebKit/Source/WebCore/editing/Editor.cpp:144 #24 WebCore::EventHandler::defaultKeyboardEventHandler (this=0x109015be8, event=0x12b2195c0) at WebKit/Source/WebCore/page/EventHandler.cpp:2658 #25 WebCore::Node::defaultEventHandler (this=0x12b3b1b10, event=0x12b2195c0) at WebKit/Source/WebCore/dom/Node.cpp:2811 #26 WebCore::EventDispatcher::dispatchEvent (this=0x7fff5fbfeb40, event=@0x7fff5fbfeb00) at WebKit/Source/WebCore/dom/EventDispatcher.cpp:346 #27 WebCore::EventDispatchMediator::dispatchEvent (this=0x7fff5fbfebb0, dispatcher=0x7fff5fbfeb40) at WebKit/Source/WebCore/dom/Event.cpp:303 #28 WebCore::EventDispatcher::dispatchEvent (node=0x12b3b1b10, mediator=@0x7fff5fbfebb0) at WebKit/Source/WebCore/dom/EventDispatcher.cpp:54 #29 WebCore::Node::dispatchEvent (this=0x12b3b1b10, event=@0x7fff5fbfec10) at WebKit/Source/WebCore/dom/Node.cpp:2717 #30 WebCore::EventTarget::dispatchEvent (this=0x12b3b1b10, event=@0x7fff5fbfed40, ec=@0x7fff5fbfedcc) at WebKit/Source/WebCore/dom/EventTarget.cpp:320 #31 WebCore::EventHandler::keyEvent (this=0x109015be8, initialKeyEvent=@0x7fff5fbfee30) at WebKit/Source/WebCore/page/EventHandler.cpp:2562 #32 WebCore::EventHandler::keyEvent (this=0x109015be8, event=0x12b2110a0) at WebKit/Source/WebCore/page/mac/EventHandlerMac.mm:129 #33 -[WebHTMLView keyDown:] (self=0x12b3a7470, _cmd=0x7fff82966400, event=0x12b2110a0) at WebKit/Source/WebKit/mac/WebView/WebHTMLView.mm:4044
> This is reproducible with Safari 5.0.5 - Today's WebKit nightly. To clarify, this is not a regression from Safari/WebKit 5.0.5.
The problem is that search & cancel buttons and innerTextElement become editable in design mode and DeleteSelectionCommand liberally remove them all :( We should modify rendererIsEditable so that we don't treat these elements as editable.
I talked with Dimitri and Levi and we agreed that the correct approach here is not to propagate designMode=true into the shadow DOM. So this crash can be fixed by one line change: --- Source/WebCore/dom/Node.cpp (revision 92006) +++ Source/WebCore/dom/Node.cpp (working copy) @@ -781,7 +781,7 @@ bool Node::rendererIsEditable(EditableLevel editableLevel) const { - if (document()->frame() && document()->frame()->page() && document()->frame()->page()->isEditable()) + if (document()->frame() && document()->frame()->page() && document()->frame()->page()->isEditable() && !shadowTreeRootNode()) return true;
Oops, forget about my last comment. That's not true at all because designMode is now set via CSS style. But RenderTextControlSingleLine::createInnerBlockStyle sets -webkit-user-modify: readonly so I don't know why nodes are editable in the shadow DOM. Need more investigation here. Does shadow DOM inherit document's style?
Created attachment 102544 [details] fixes the bug
Comment on attachment 102544 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=102544&action=review > Source/WebCore/css/CSSStyleSelector.cpp:1940 > + if (isAtShadowBoundary(e)) I wonder if you could just test for e->isShadowRoot(). I think that would make the fix more correct in the sense you mention in the ChangeLog.
Comment on attachment 102544 [details] fixes the bug Found a much better solution.
Created attachment 102549 [details] fixes the bug with much less hack
Comment on attachment 102549 [details] fixes the bug with much less hack View in context: https://bugs.webkit.org/attachment.cgi?id=102549&action=review > Source/WebCore/css/CSSStyleSelector.cpp:1365 > + // Don't propagate user-modify into shadow DOM I guess this comment just repeats what the code says. Will remove it before landing the patch if r+ed.
Comment on attachment 102549 [details] fixes the bug with much less hack View in context: https://bugs.webkit.org/attachment.cgi?id=102549&action=review >> Source/WebCore/css/CSSStyleSelector.cpp:1365 >> + // Don't propagate user-modify into shadow DOM > > I guess this comment just repeats what the code says. Will remove it before landing the patch if r+ed. Instead of removing it you could put a why comment in. Something like “Even if surrounding content is user-editable, shadow DOM should act as a single unit, and not necessarily be editable.”
Committed r92139: <http://trac.webkit.org/changeset/92139>
(In reply to comment #11) > Instead of removing it you could put a why comment in. Something like “Even if surrounding content is user-editable, shadow DOM should act as a single unit, and not necessarily be editable.” Will do now.
Huh... I somehow landed the patch with the comment in it. Anyway, updated to what Darin suggested in http://trac.webkit.org/changeset/92140.