Scrollbars don't currently handle right clicks, but we are showing the context menu when they are right clicked. This is not desired at least in GTK+ and I've checked that other applications in Mac don't show the context menu when right clicking on a scrollbar.
Created attachment 269890 [details] Patch
Comment on attachment 269890 [details] Patch Isn't this testable?
Created attachment 270202 [details] Add a test
Comment on attachment 270202 [details] Add a test Attachment 270202 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/755258 New failing tests: fast/events/contextmenu-on-scrollbars.html
Created attachment 270204 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
I'm surprised it only crashed in mac wk2, could someone with a mac check if the tests fails even without the patch?
Comment on attachment 270202 [details] Add a test This looks like it introduced crashes in WebKit2 tests on Mac.
(In reply to comment #7) > Comment on attachment 270202 [details] > Add a test > > This looks like it introduced crashes in WebKit2 tests on Mac. Could your confirm that the test doesn't crash with current trunk (even if it fails, of course). I'm surprised it only fails in wk2, and the changes introduced are cross-platform. The only thing the patch does differently is calling view->scrollbarAtPoint(event.position()). And view is null checked already. The other thing is that I remove the !mouseEvent.scrollbar() condition from the if below, because we are now returning early when having a scrollbar.
(In reply to comment #8) > Could your confirm that the test doesn't crash with current trunk (even if > it fails, of course). I have no idea. I was just looking at EWS output.
My guess is the crash is unrelated to your code change, and you just happened to find some test that crashes JSC. CRASHING TEST: fast/events/contextmenu-on-scrollbars.html Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebKit 0x000000010bd528a9 WKArrayGetSize + 9 1 WebKitTestRunnerInjectedBundle 0x00000001172386f7 0x117228000 + 67319 2 com.apple.JavaScriptCore 0x000000010cf49bab long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState*) + 571 (APICallbackFunction.h:61) 3 com.apple.JavaScriptCore 0x000000010d071343 JSC::LLInt::setUpCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind, JSC::JSValue, JSC::LLIntCallLinkInfo*) + 595 (LLIntSlowPaths.cpp:1110) 4 com.apple.JavaScriptCore 0x000000010d078157 llint_entry + 23679 5 com.apple.JavaScriptCore 0x000000010d0722f5 vmEntryToJavaScript + 299 6 com.apple.JavaScriptCore 0x000000010cefc9ae JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 158 (JITCode.cpp:81) 7 com.apple.JavaScriptCore 0x000000010ce7b15e JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) + 10558 (Interpreter.cpp:972) 8 com.apple.JavaScriptCore 0x000000010cb27d91 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 593 (Completion.cpp:105) 9 com.apple.WebCore 0x000000010e56bef5 WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&, WebCore::ExceptionDetails*) + 309 (JSMainThreadExecState.h:80) 10 com.apple.WebCore 0x000000010e56c140 WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&, WebCore::ExceptionDetails*) + 48 (ScriptController.cpp:180) 11 com.apple.WebCore 0x000000010e5721d4 WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&) + 260 (ScriptElement.cpp:310) 12 com.apple.WebCore 0x000000010e570ce5 WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) + 1061 (StdLibExtras.h:350) 13 com.apple.WebCore 0x000000010dd591d8 WebCore::HTMLScriptRunner::runScript(WebCore::Element*, WTF::TextPosition const&) + 344 (ScriptElement.h:59) 14 com.apple.WebCore 0x000000010dd59030 WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element>, WTF::TextPosition const&) + 48 (HTMLScriptRunner.cpp:189) 15 com.apple.WebCore 0x000000010dcf5f86 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() + 86 (StdLibExtras.h:350) 16 com.apple.WebCore 0x000000010dcf604d WebCore::HTMLDocumentParser::canTakeNextToken(WebCore::HTMLDocumentParser::SynchronousMode, WebCore::PumpSession&) + 93 (HTMLDocumentParser.cpp:214) 17 com.apple.WebCore 0x000000010dcf5c40 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) + 544 (HTMLDocumentParser.cpp:252) 18 com.apple.WebCore 0x000000010dcf6990 WebCore::HTMLDocumentParser::append(WTF::RefPtr<WTF::StringImpl>&&) + 736 (DocumentParser.h:71) 19 com.apple.WebCore 0x000000010da88725 WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter&, char const*, unsigned long) + 117 (StdLibExtras.h:350) 20 com.apple.WebCore 0x000000010dadd731 WebCore::DocumentLoader::commitData(char const*, unsigned long) + 657 (DocumentLoader.cpp:890) 21 com.apple.WebKit 0x000000010bc606c6 WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 50 22 com.apple.WebCore 0x000000010dadf6d1 WebCore::DocumentLoader::commitLoad(char const*, int) + 145 (DocumentLoader.h:229) 23 com.apple.WebCore 0x000000010d8eaa70 WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, unsigned int) + 160 (CachedResourceClientWalker.h:51) 24 com.apple.WebCore 0x000000010d8ea941 WebCore::CachedRawResource::addDataBuffer(WebCore::SharedBuffer&) + 145 (CachedRawResource.cpp:70) 25 com.apple.WebCore 0x000000010e6b155a WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, WTF::PassRefPtr<WebCore::SharedBuffer>, long long, WebCore::DataPayloadType) + 218 (SubresourceLoader.cpp:300) 26 com.apple.WebCore 0x000000010e6b1443 WebCore::SubresourceLoader::didReceiveData(char const*, unsigned int, long long, WebCore::DataPayloadType) + 35 (StdLibExtras.h:350) 27 com.apple.WebKit 0x000000010bd287ff WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::MessageDecoder&) + 291 28 com.apple.WebKit 0x000000010bafc5bd IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >) + 127 29 com.apple.WebKit 0x000000010bafecc4 IPC::Connection::dispatchOneMessage() + 126 30 com.apple.JavaScriptCore 0x000000010d2c9fd5 WTF::RunLoop::performWork() + 437 (functional:1742) 31 com.apple.JavaScriptCore 0x000000010d2ca382 WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39) 32 com.apple.CoreFoundation 0x00007fff97fa6a01 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 33 com.apple.CoreFoundation 0x00007fff97f98b8d __CFRunLoopDoSources0 + 269 34 com.apple.CoreFoundation 0x00007fff97f981bf __CFRunLoopRun + 927 35 com.apple.CoreFoundation 0x00007fff97f97bd8 CFRunLoopRunSpecific + 296 36 com.apple.HIToolbox 0x00007fff9842856f RunCurrentEventLoopInMode + 235 37 com.apple.HIToolbox 0x00007fff984282ea ReceiveNextEventCommon + 431 38 com.apple.HIToolbox 0x00007fff9842812b _BlockUntilNextEventMatchingListInModeWithFilter + 71 39 com.apple.AppKit 0x00007fff8c4718ab _DPSNextEvent + 978 40 com.apple.AppKit 0x00007fff8c470e58 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 346 41 com.apple.AppKit 0x00007fff8c466af3 -[NSApplication run] + 594 42 com.apple.AppKit 0x00007fff8c3e3244 NSApplicationMain + 1832 43 libxpc.dylib 0x00007fff90bb8928 _xpc_objc_main + 793 44 libxpc.dylib 0x00007fff90bba030 xpc_main + 490 45 com.apple.WebKit.WebContent.Development 0x000000010bab4e78 main + 422 (XPCServiceMain.mm:114) 46 libdyld.dylib 0x00007fff90c0a5c9 start + 1
Ok, I know what the problem is. The crash is in WTR, when handling contextClick. GTK and EFL use WKBundlePageCopyContextMenuItems that always returns a valid array, even if it's empty when there's not contexct menu, but other ports use WKBundlePageCopyContextMenuAtPointInWindow() that can return nullptr. The returned value is used below without any null check. I don't know why we need different code for EFL and GTK here, and I don't know why WebPage::contextMenuAtPointInWindow() can return nullptr, but WebPage::contextMenu() doesn't. But this is an exiting issue in any case so I'll open a new bug.
Comment on attachment 270202 [details] Add a test In that case, r=me, good job as usual.
Created attachment 270637 [details] Patch for landing
Committed r196112: <http://trac.webkit.org/changeset/196112>