[chromium] Trigger context menu for long press gesture

• Component: New Bugs
• Status: RESOLVED FIXED

[Varun Jain]

[chromium] Trigger context menu for long press gesture

[Varun Jain]

Created attachment 140792 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 140792 [details]
Patch

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

How do we test this?

> Source/WebCore/page/EventHandler.cpp:2603
> +#if OS(WINDOWS)

Why is the Win difference needed here?

[Adam Barth]

Comment on attachment 140792 [details]
Patch

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

>> Source/WebCore/page/EventHandler.cpp:2603
>> +#if OS(WINDOWS)
> 
> Why is the Win difference needed here?

I'm not sure this is the right place to put this code, but I suspect that the difference is that there's a difference in when to show context menus on mac and windows.  I think mac shows the context menu on mouse down and windows shows them on mouse up.  I imagine this is analogous.

[Varun Jain]

Created attachment 140975 [details]
Patch

[Varun Jain]

(In reply to comment #2)
> (From update of attachment 140792 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=140792&action=review
> 
> How do we test this?

Added layout test

> 
> > Source/WebCore/page/EventHandler.cpp:2603
> > +#if OS(WINDOWS)
> 
> Why is the Win difference needed here?

Adam answered this part. Thanks Adam!
Regarding Adam's concern about the placement of this code, I think this is in line with EventHandler::sendContextMenuEventForKey()
Another motivation to put this in EventHandler is that other platforms can call this to show context menu on different gestures.

[Varun Jain]

ping... eric, abarth: I have addressed all comments. Please take another look.

(In reply to comment #5)
> (In reply to comment #2)
> > (From update of attachment 140792 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=140792&action=review
> > 
> > How do we test this?
> 
> Added layout test
> 
> > 
> > > Source/WebCore/page/EventHandler.cpp:2603
> > > +#if OS(WINDOWS)
> > 
> > Why is the Win difference needed here?
> 
> Adam answered this part. Thanks Adam!
> Regarding Adam's concern about the placement of this code, I think this is in line with EventHandler::sendContextMenuEventForKey()
> Another motivation to put this in EventHandler is that other platforms can call this to show context menu on different gestures.

[Adam Barth]

Comment on attachment 140975 [details]
Patch

This looks good.  Thanks.

[WebKit Review Bot]

Comment on attachment 140975 [details]
Patch

Clearing flags on attachment: 140975

Committed r116671: <http://trac.webkit.org/changeset/116671>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.