If CONTEXT_MENU isn't enabled there will be a call to "sendContextMenuEventForGesture(gestureEvent);" that is wrapped with "#if ENABLE(CONTEXT_MENU)" and the code won't compile.
Created attachment 167984 [details] Proposed patch Enable EventHandler::handleGestureLongPress only if CONTEXT_MENU is set
Comment on attachment 167984 [details] Proposed patch Attachment 167984 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14238708
Comment on attachment 167984 [details] Proposed patch Attachment 167984 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14248495
Comment on attachment 167984 [details] Proposed patch Attachment 167984 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14245514
Comment on attachment 167984 [details] Proposed patch Attachment 167984 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14252183
Comment on attachment 167984 [details] Proposed patch Attachment 167984 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14245517
Comment on attachment 167984 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=167984&action=review > Source/WebCore/page/EventHandler.cpp:2573 > +#if ENABLE(CONTEXT_MENU) This ifdef is in the wrong place. Shouldn't it be protecting the sendContextMenuEventForGesture() call?
Created attachment 167997 [details] Another proposed patch
(In reply to comment #7) > (From update of attachment 167984 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167984&action=review > > > Source/WebCore/page/EventHandler.cpp:2573 > > +#if ENABLE(CONTEXT_MENU) > > This ifdef is in the wrong place. Shouldn't it be protecting the sendContextMenuEventForGesture() call? But in this case we've to return something. If you're not ANDROID the only line remaining is this sendContextMenuEventForGesture(). Should we just return "false" (after moving ENABLE(CONTEXT_MENUS) to the proper place)?
Comment on attachment 167984 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=167984&action=review >>> Source/WebCore/page/EventHandler.cpp:2573 >>> +#if ENABLE(CONTEXT_MENU) >> >> This ifdef is in the wrong place. Shouldn't it be protecting the sendContextMenuEventForGesture() call? > > But in this case we've to return something. If you're not ANDROID the only line remaining is this sendContextMenuEventForGesture(). Should we just return "false" (after moving ENABLE(CONTEXT_MENUS) to the proper place)? Yes. But sendContextMenuEventForGesture() is only declared if we have CONTEXT_MENUS (see EventHandler.h), so you need to guard it. My understanding from the event handling is that if it's not defined you can simply return false here. So add an else case for the #if ENABLE(CONTEXT_MENUS) returning false should be enough.
Created attachment 168010 [details] Another proposed patch
Comment on attachment 168010 [details] Another proposed patch Let's wait the EWS just in case :-)
Created attachment 168020 [details] Patch for landing
Created attachment 168022 [details] Patch for landing
Comment on attachment 168022 [details] Patch for landing Rejecting attachment 168022 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: it2/ChangeLog Auto-merging Source/WebKit2/win/WebKit2.def Failed to merge in the changes. Patch failed at 0001 Needs internal API to return distributed nodes for InsertionPoint When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. Full output: http://queues.webkit.org/results/14260093
Committed r130930: <http://trac.webkit.org/changeset/130930>