Context menu doesn't work on Webkit Mac

[GoogleCodeExporter]

Using a Mac laptop without a mouse, the standard way of bringing up the context 
menu is pressing control and clicking. Webkit on a Mac doesn't show up the 
context menu if you control-click because the current jquery.contextMenu.js 
considers the only valid way of bringing up the context menu is with a right 
right mouse click. Firefox seems so emulate this behavior, because it does show 
up on FF.

What steps will reproduce the problem?
1. Try control clicking on the canvas with a Mac under Chrome/Safari

What is the expected output? What do you see instead?
Expected a context menu, got nothing

In what browser did you experience this problem? (ALL, Firefox, Opera, etc)
Only tested Chrome, but I suspect Safari too.

In what version of SVG-edit does the problem occur? (Latest trunk, 2.5.1,
etc)

All including latest trunk

The right way of doing this would be to use the contextmenu event 
http://www.quirksmode.org/dom/events/contextmenu.html, however I don't know if 
this would break other things, so I'm including a patch that just respects the 
current implementation and additionally listens for the contextmenu event.

Please provide any additional information below.

Original issue reported on code.google.com by duopi...@gmail.com on 5 Mar 2012 at 6:06

Attachments:

• jquery.contextMenu.js

[GoogleCodeExporter]

Hey duopixel,

thanks! But I'm pretty sure that is not a patch. That's a whole file! Check 
other issues for example patches.

Original comment by asyazwan on 5 Mar 2012 at 7:51

• Changed state: Accepted

[GoogleCodeExporter]

Sorry, this is the first time I submit a patch. Is this one right?

Original comment by duopi...@gmail.com on 5 Mar 2012 at 2:49

Attachments:

• jquery.contextMenu.patch

[GoogleCodeExporter]

Yes, that seems more like a patch.

Easiest to generate should be by going into your svg-edit svn directory and 
modify your file, then do:
svn diff

or output to file:
svn diff > 919.patch

done!
---
Raising the priority as this seems pretty important for macs with no option but 
to Cmd+Click.

Original comment by asyazwan on 5 Mar 2012 at 4:26

• Added labels: NeededFor-2.6, Priority-High
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Btw, two-fingers on the trackpad and then clicking is my standard way of 
"right-clicking" on a Mac and this brings up the context menu.  Nonetheless, I 
will look at this patch today, thanks!

Original comment by codedr...@gmail.com on 17 Mar 2012 at 4:08

[GoogleCodeExporter]

Thanks a lot for this patch!  Merged in r2060. :)

Next time, please use "svn diff" to create your patch.  I had to manually patch 
this file because it said "missing header for unified diff at line 2 of patch". 
 I've also left a message at the original developer's site to let them know we 
have modified their extension in case they want to pick up our changes.

Original comment by codedr...@gmail.com on 17 Mar 2012 at 5:45

• Changed state: Fixed

[GoogleCodeExporter]

Thanks! This is the first time I submit a patch anywhere, I'll make sure to use 
the built in tools next time. Also: great tip on the double finger click, all 
these years wasted reaching for the control key...

Original comment by duopi...@gmail.com on 17 Mar 2012 at 5:53

[GoogleCodeExporter]

I'm pretty sure this patch introduced a new bug.

Try right-clicking twice and left-clicking. Context menu will popup. It seems 
any right-clicking more than once will do this.

Not really a blocker, but I just tested with a pre-patch version and it's ok.

Do you mind looking into it, duopixel?

Original comment by asyazwan on 19 Mar 2012 at 6:25

• Changed state: Accepted
• Added labels: Priority-Medium
• Removed labels: NeededFor-2.6, Priority-High

[GoogleCodeExporter]

I'm not able to replicate, can you let me know your browser/os?

Original comment by duopi...@gmail.com on 19 Mar 2012 at 7:06

[GoogleCodeExporter]

FF 11 / Chrome 17 on Windows 7 x64.

On FF, it only takes 1 right-click to reproduce.

Original comment by asyazwan on 19 Mar 2012 at 11:58

[GoogleCodeExporter]

It's binded to two different events, so the code is executed twice, but I don't 
know why this is only evident on Windows. 

I had to rework the code so it uses the real contextmenu event, since Opera 
doesn't pass the mouseup event if the event originated from contextmenu.

I don't have a windows machine to test this, mind giving it a try?

Original comment by duopi...@gmail.com on 19 Mar 2012 at 4:06

Attachments:

• contentext_menu.diff

[GoogleCodeExporter]

Hi,

Good news is it works as expected on Chrome.

Bad news is it now requires 2 right clicks to open the menu on FF 11.

Original comment by asyazwan on 20 Mar 2012 at 12:48

[GoogleCodeExporter]

I have a question though. Why is it necessary to bind everytime any click 
occurs? Doesn't that seems inefficient? Wouldn't checking if event is bound 
first be better?

Original comment by asyazwan on 20 Mar 2012 at 12:51

[GoogleCodeExporter]

It's not binded on click as far as I can tell. It does execute on every click 
done on the workspace and layers area.

I think this problem is beyond my abilities, I'm submitting a final patch that 
does ugly browser sniffing to bring up the context menu if control + click && 
isMac, but feel free to revert to the original version is you think it's too 
hacky.

Original comment by duopi...@gmail.com on 20 Mar 2012 at 4:34

Attachments:

• context-menu.diff

[GoogleCodeExporter]

Yeah I think it is best to introduce a hack just for Mac, rather than risking 
malfunction in all other browsers/platform.

FYI we have browser utility functions under namespace svgedit.browser.

Are you up for adding isMac in browser.js and use it in your patch?

On a related note, from quickly going through the function I found out that 
contextmenu event is bound twice. Could this be the root? You can check with 
$('#workarea').data('events') on your console right after loading.

Original comment by asyazwan on 20 Mar 2012 at 6:26

[GoogleCodeExporter]

Here is the patch with the added isMac method to svgedit.browser and the fix 
for ctrl + click.

If the second contextmenu binding showed up in my previous patch, in my working 
copy I merged the events and it was still misbehaving.

Original comment by duopi...@gmail.com on 20 Mar 2012 at 7:03

Attachments:

• context-menu.diff

[GoogleCodeExporter]

Alright, patch applied in r2065.

Thank you very much. :)

Original comment by asyazwan on 20 Mar 2012 at 9:00

• Changed state: Fixed