WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
65362
Search field in designMode causes a crash
https://bugs.webkit.org/show_bug.cgi?id=65362
Summary
Search field in designMode causes a crash
Kent Tamura
Reported
2011-07-29 00:40:19 PDT
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.
Attachments
fixes the bug
(4.98 KB, patch)
2011-08-01 12:33 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
fixes the bug with much less hack
(5.49 KB, patch)
2011-08-01 13:40 PDT
,
Ryosuke Niwa
dglazkov
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
noel gordon
Comment 1
2011-07-29 01:21:40 PDT
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
Alexey Proskuryakov
Comment 2
2011-07-29 10:47:39 PDT
> 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.
Ryosuke Niwa
Comment 3
2011-07-29 12:06:41 PDT
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.
Ryosuke Niwa
Comment 4
2011-07-29 14:12:02 PDT
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;
Ryosuke Niwa
Comment 5
2011-07-29 15:07:50 PDT
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?
Ryosuke Niwa
Comment 6
2011-08-01 12:33:21 PDT
Created
attachment 102544
[details]
fixes the bug
Dimitri Glazkov (Google)
Comment 7
2011-08-01 12:37:39 PDT
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.
Ryosuke Niwa
Comment 8
2011-08-01 13:35:34 PDT
Comment on
attachment 102544
[details]
fixes the bug Found a much better solution.
Ryosuke Niwa
Comment 9
2011-08-01 13:40:23 PDT
Created
attachment 102549
[details]
fixes the bug with much less hack
Ryosuke Niwa
Comment 10
2011-08-01 13:42:21 PDT
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.
Darin Adler
Comment 11
2011-08-01 13:54:09 PDT
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.”
Ryosuke Niwa
Comment 12
2011-08-01 14:08:22 PDT
Committed
r92139
: <
http://trac.webkit.org/changeset/92139
>
Ryosuke Niwa
Comment 13
2011-08-01 14:09:19 PDT
(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.
Ryosuke Niwa
Comment 14
2011-08-01 14:11:35 PDT
Huh... I somehow landed the patch with the comment in it. Anyway, updated to what Darin suggested in
http://trac.webkit.org/changeset/92140
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug