Bug 145981 – Crash when right clicking in input box with -webkit-user-select: none

• Component: Forms
• Status: RESOLVED FIXED

[Bem Jones-Bey]

Created attachment 254888 [details]
Test Case

Load the attached test case, then right click in the input box. Watch the renderer crash:

Note that if you do type in the box, only one character shows up, and doesn't seem to be able to be deleted. But once the box has been left clicked on or typed in, a crash will no longer occur.

Stack:

* thread #1: tid = 0xae932b, 0x000000011830247f WebCore`WebCore::Node::getFlag(this=0x0000000000000000, mask=HasRareDataFlag) const + 15 at Node.h:631, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x18)
  * frame #0: 0x000000011830247f WebCore`WebCore::Node::getFlag(this=0x0000000000000000, mask=HasRareDataFlag) const + 15 at Node.h:631
    frame #1: 0x000000011830243a WebCore`WebCore::Node::hasRareData(this=0x0000000000000000) const + 26 at Node.h:658
    frame #2: 0x00000001183023d9 WebCore`WebCore::Node::renderer(this=0x0000000000000000) const + 25 at Node.h:441
    frame #3: 0x0000000118a3095f WebCore`WebCore::Editor::hasBidiSelection(this=0x00007f8520c29900) const + 447 at Editor.cpp:712
    frame #4: 0x0000000118648adb WebCore`WebCore::ContextMenuController::populate(this=0x0000000121fcf0b8) + 15547 at ContextMenuController.cpp:1098
    frame #5: 0x00000001186448af WebCore`WebCore::ContextMenuController::handleContextMenuEvent(this=0x0000000121fcf0b8, event=0x0000000121f586e0) + 799 at ContextMenuController.cpp:105
    frame #6: 0x00000001199432e0 WebCore`WebCore::Node::defaultEventHandler(this=0x0000000121fef9c0, event=0x0000000121f586e0) + 592 at Node.cpp:2088
    frame #7: 0x000000011a2555c9 WebCore`WebCore::TextControlInnerTextElement::defaultEventHandler(this=0x0000000121fef9c0, event=0x0000000121f586e0) + 185 at TextControlInnerElements.cpp:111
    frame #8: 0x0000000118a872a6 WebCore`WebCore::callDefaultEventHandlersInTheBubblingOrder(event=0x0000000121f586e0, path=0x00007fff51913ee8) + 102 at EventDispatcher.cpp:257
    frame #9: 0x0000000118a86c39 WebCore`WebCore::EventDispatcher::dispatchEvent(origin=0x0000000121fef9c0, prpEvent=PassRefPtr<WebCore::Event> at 0x00007fff51914040) + 1065 at EventDispatcher.cpp:368
    frame #10: 0x0000000119942bbd WebCore`WebCore::Node::dispatchEvent(this=0x0000000121fef9c0, event=<unavailable>) + 45 at Node.cpp:2011
    frame #11: 0x0000000118a5749f WebCore`WebCore::Element::dispatchMouseEvent(this=0x0000000121fef9c0, platformEvent=0x00007fff51914598, eventType=0x0000000120038d70, detail=0, relatedTarget=0x0000000000000000) + 559 at Element.cpp:264
    frame #12: 0x0000000118a9641f WebCore`WebCore::EventHandler::dispatchMouseEvent(this=0x00000001217e5000, eventType=0x0000000120038d70, targetNode=0x0000000121fef9c0, (null)=true, clickCount=0, platformMouseEvent=0x00007fff51914598, setUnder=false) + 207 at EventHandler.cpp:2582
    frame #13: 0x0000000118a9af5f WebCore`WebCore::EventHandler::sendContextMenuEvent(this=0x00000001217e5000, event=0x00007fff51914598) + 495 at EventHandler.cpp:2833
    frame #14: 0x000000011a31a5cc WebCore`WebCore::UserInputBridge::handleContextMenuEvent(this=0x00007f8520c135b0, mouseEvent=0x00007fff51914598, frame=0x00000001217e6000, (null)=User) + 44 at UserInputBridge.cpp:74
    frame #15: 0x00000001138338aa WebKit`WebKit::handleContextMenuEvent(platformMouseEvent=0x00007fff51914598, page=0x00007f8521012410) + 298 at WebPage.cpp:1918
    frame #16: 0x0000000113828cef WebKit`WebKit::handleMouseEvent(mouseEvent=0x00007fff519147c0, page=0x00007f8521012410, onlyUpdateScrollbars=false) + 271 at WebPage.cpp:1944
    frame #17: 0x0000000113828b79 WebKit`WebKit::WebPage::mouseEvent(this=0x00007f8521012410, mouseEvent=0x00007fff519147c0) + 521 at WebPage.cpp:2006
    frame #18: 0x00000001138a43ef WebKit`void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebMouseEvent const&), std::__1::tuple<WebKit::WebMouseEvent>, 0ul>(object=0x00007f8521012410, function=0x0000000113828970, args=0x00007fff519147c0, (null)=index_sequence<0> at 0x00007fff51914700)(WebKit::WebMouseEvent const&), std::__1::tuple<WebKit::WebMouseEvent>&&, std::index_sequence<0ul>) + 159 at HandleMessage.h:16
    frame #19: 0x00000001138a4348 WebKit`void IPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebMouseEvent const&), std::__1::tuple<WebKit::WebMouseEvent>, std::make_index_sequence<1ul> >(args=0x00007fff519147c0, object=0x00007f8521012410, function=0x0000000113828970)(WebKit::WebMouseEvent const&)) + 88 at HandleMessage.h:22
    frame #20: 0x000000011388bf94 WebKit`void IPC::handleMessage<Messages::WebPage::MouseEvent, WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::WebMouseEvent const&)>(decoder=0x0000000121fe46e8, object=0x00007f8521012410, function=0x0000000113828970)(WebKit::WebMouseEvent const&)) + 244 at HandleMessage.h:92
    frame #21: 0x00000001138860e6 WebKit`WebKit::WebPage::didReceiveWebPageMessage(this=0x00007f8521012410, connection=0x00000001217fa000, decoder=0x0000000121fe46e8) + 1574 at WebPageMessageReceiver.cpp:179
    frame #22: 0x000000011382da5b WebKit`WebKit::WebPage::didReceiveMessage(this=0x00007f8521012410, connection=0x00000001217fa000, decoder=0x0000000121fe46e8) + 379 at WebPage.cpp:3630
    frame #23: 0x000000011382daa7 WebKit`non-virtual thunk to WebKit::WebPage::didReceiveMessage(this=0x00007f8521012420, connection=0x00000001217fa000, decoder=0x0000000121fe46e8) + 55 at WebPage.cpp:3631
    frame #24: 0x000000011328d61d WebKit`IPC::MessageReceiverMap::dispatchMessage(this=0x00007f8521814c58, connection=0x00000001217fa000, decoder=0x0000000121fe46e8) + 461 at MessageReceiverMap.cpp:87
    frame #25: 0x00000001139add9d WebKit`WebKit::WebProcess::didReceiveMessage(this=0x00007f8521814c00, connection=0x00000001217fa000, decoder=0x0000000121fe46e8) + 61 at WebProcess.cpp:617
    frame #26: 0x000000011313f3f3 WebKit`IPC::Connection::dispatchMessage(this=0x00000001217fa000, decoder=0x0000000121fe46e8) + 51 at Connection.cpp:870
    frame #27: 0x00000001131378e0 WebKit`IPC::Connection::dispatchMessage(this=0x00000001217fa000, message=unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> > at 0x00007fff519169c8) + 416 at Connection.cpp:893
    frame #28: 0x000000011313f9ef WebKit`IPC::Connection::dispatchOneMessage(this=0x00000001217fa000) + 1519 at Connection.cpp:921
    frame #29: 0x000000011314126d WebKit`IPC::Connection::enqueueIncomingMessage(this=0x00007f8520f4d168)::$_9::operator()() const + 29 at Connection.cpp:864
    frame #30: 0x000000011314123c WebKit`std::__1::__function::__func<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9, std::__1::allocator<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9>, void ()>::operator()() [inlined] decltype(__f=0x00007f8520f4d168)::$_9&>(fp)(std::__1::forward<>(fp0))) std::__1::__invoke<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9&>(IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9&&&) + 60 at __functional_base:413
    frame #31: 0x000000011314122b WebKit`std::__1::__function::__func<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9, std::__1::allocator<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9>, void ()>::operator(this=0x00007f8520f4d160)() + 43 at functional:1370
    frame #32: 0x00000001166da92a JavaScriptCore`std::__1::function<void ()>::operator(this=0x00007fff51916ec0)() const + 26 at functional:1756
    frame #33: 0x0000000116be5302 JavaScriptCore`WTF::RunLoop::performWork(this=0x0000000121ff9000) + 306 at RunLoop.cpp:104
    frame #34: 0x0000000116be65b4 JavaScriptCore`WTF::RunLoop::performWork(context=0x0000000121ff9000) + 36 at RunLoopCF.cpp:38
    frame #35: 0x00007fff8ec86681 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
    frame #36: 0x00007fff8ec7880d CoreFoundation`__CFRunLoopDoSources0 + 269
    frame #37: 0x00007fff8ec77e3f CoreFoundation`__CFRunLoopRun + 927
    frame #38: 0x00007fff8ec77858 CoreFoundation`CFRunLoopRunSpecific + 296
    frame #39: 0x00007fff9216aaef HIToolbox`RunCurrentEventLoopInMode + 235
    frame #40: 0x00007fff9216a86a HIToolbox`ReceiveNextEventCommon + 431
    frame #41: 0x00007fff9216a6ab HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 71
    frame #42: 0x00007fff8a338f81 AppKit`_DPSNextEvent + 964
    frame #43: 0x00007fff8a338730 AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 194
    frame #44: 0x00007fff8a32c593 AppKit`-[NSApplication run] + 594
    frame #45: 0x00007fff8a317a14 AppKit`NSApplicationMain + 1832
    frame #46: 0x00007fff90f1eef2 libxpc.dylib`_xpc_objc_main + 793
    frame #47: 0x00007fff90f20a9d libxpc.dylib`xpc_main + 490
    frame #48: 0x000000010e2e8197 com.apple.WebKit.WebContent.Development`main(argc=1, argv=0x00007fff519187f0) + 39 at XPCServiceMain.Development.mm:170
    frame #49: 0x00007fff935345c9 libdyld.dylib`start + 1

[Radar WebKit Bug Importer]

<rdar://problem/22441925>

[Jiewen Tan]

Created attachment 264368 [details]
Patch

[Jiewen Tan]

The crash has been fixed, but I am not sure whether the behavior the report described should be abnormal or not. Therefore, I suggest the reporter to file another bug report about the "abnormal" behavior.

[Bem Jones-Bey]

Comment on attachment 264368 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264368&action=review

I don't know enough about the context menu stuff to give a real review, but thanks for fixing this! I'll take a look once this has landed and file a new bug on the weird input behavior if it persists.

> LayoutTests/editing/selection/minimal-user-select-crash-expected.txt:1
> +EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification

There may not be a way around it, but it is unfortunate that this ends up in the expected result, since it'll probably cause the test to fail on non-Mac ports.

[Jiewen Tan]

Comment on attachment 264368 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264368&action=review

>> LayoutTests/editing/selection/minimal-user-select-crash-expected.txt:1
>> +EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> 
> There may not be a way around it, but it is unfortunate that this ends up in the expected result, since it'll probably cause the test to fail on non-Mac ports.

Currently, all the layout tests under editing will include things like this. I have to follow the rule. Maybe you can ask Enrica for more detailed explanation.

[David Kilzer (:ddkilzer)]

(In reply to comment #5)
> Comment on attachment 264368 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264368&action=review
> 
> >> LayoutTests/editing/selection/minimal-user-select-crash-expected.txt:1
> >> +EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> > 
> > There may not be a way around it, but it is unfortunate that this ends up in the expected result, since it'll probably cause the test to fail on non-Mac ports.
> 
> Currently, all the layout tests under editing will include things like this.
> I have to follow the rule. Maybe you can ask Enrica for more detailed
> explanation.

There may be a way to disable editing delegates on a per-test basis via window.internal function.

Otherwise, we can just land separate results for the other ports.

[Darin Adler]

Comment on attachment 264368 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264368&action=review

Please rethink the regression test.

>>>> LayoutTests/editing/selection/minimal-user-select-crash-expected.txt:1
>>>> +EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>>> 
>>> There may not be a way around it, but it is unfortunate that this ends up in the expected result, since it'll probably cause the test to fail on non-Mac ports.
>> 
>> Currently, all the layout tests under editing will include things like this. I have to follow the rule. Maybe you can ask Enrica for more detailed explanation.
> 
> There may be a way to disable editing delegates on a per-test basis via window.internal function.
> 
> Otherwise, we can just land separate results for the other ports.

What turns this on is code that says:

    testRunner.dumpEditingCallbacks();

It’s not because the test is in the editing directory, but because it’s using editing/editing.js and calling runEditingTest. I think there should be a way to change things so it won’t dump the editing callbacks. I am not sure this test needs to use runEditingTest.

[Jiewen Tan]

Created attachment 264824 [details]
Patch

[Enrica Casucci]

Comment on attachment 264368 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264368&action=review

Please address my comment for the ChangeLog before landing. The change looks ok.

> Source/WebCore/ChangeLog:13
> +        Selection could point to non visible position with -webkit-user-select: none.

I think the correct comment is that there is no visible position. The problem is not that the position is not visible, but that a visible position cannot be created because of the style that makes it non selectable.

[Enrica Casucci]

Comment on attachment 264824 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264824&action=review

Please address my comment to the ChangeLog before committing the patch. The change looks ok.

> Source/WebCore/ChangeLog:13
> +        Selection could point to non visible position with -webkit-user-select: none.

This comment is not correct. It is not that the position is non visible, but rather that a visible position cannot be created because of the style that doesn't allow the selection.

[Jiewen Tan]

Created attachment 265110 [details]
Patch for committing

[WebKit Commit Bot]

Comment on attachment 265110 [details]
Patch for committing

Clearing flags on attachment: 265110

Committed r192191: <http://trac.webkit.org/changeset/192191>