Closed manuel-woelker closed 12 years ago
olivernn
commented
12 years ago What version of Davis are you seeing this problem in?
Using the demo app at http://davis-example.heroku.com/ I can open a link in a new tab without any issues in all browsers on my mac. Perhaps this is a Windows specific issue? What browsers did you notice this problem in?
manuel-woelker
commented
12 years ago Sorry, I should have mentioned in the pull request. I tried some browsers with the following results.
New tabs are opened with:
New tabs are not opening for me on:
I also found the following stackoverflow questions regarding this issue:
Thanks for the feedback!
Valloric
commented
12 years ago Same problem here with Chrome 23.0.1271.26 beta (and earlier); confirmed that the patch fixes the issue. It would be great if this could be merged some time soon.
olivernn
commented
12 years ago @Valloric what os are you using?
If this is still an issue then I'll take a look at getting this merged in quickly, would appreciate some help testing this the fix out on some windows and linux machines where possible, @manuel-woelker is this something you would be able to help with too?
Valloric
commented
12 years ago Tested and confirmed as a problem on Ubuntu 12.04 and Mac OS X Mountain Lion. I've already listed the browser versions (basically Chrome latest and older). Note that on Mac, the shortcut is Command-Click, not Control-Click, so the patch doesn't fix the problem on Macs (just confirmed).
Just tested on Safari Version 6.0.1 (8536.26.14), Mac OS X Mountain Lion and Command-Click on a link does not open in a new tab there either (same with Chrome).
On Thu, Oct 18, 2012 at 3:01 AM, Oliver Nightingale < notifications@github.com> wrote:
@Valloric https://github.com/Valloric what os are you using?
If this is still an issue then I'll take a look at getting this merged in quickly, would appreciate some help testing this the fix out on some windows and linux machines where possible, @manuel-woelkerhttps://github.com/manuel-woelkeris this something you would be able to help with too?
— Reply to this email directly or view it on GitHubhttps://github.com/olivernn/davis.js/pull/40#issuecomment-9559209.
Valloric
commented
12 years ago I'll be happy to test out whatever fix you come up with. I have access to Ubuntu and Mac machines and Chrome, Safari and Firefox.
Valloric
commented
12 years ago The IF statement should probably check all the modifier keys; event.metaKey will be true when Command is held down on a Mac (except in old versions of IE, AFAICT). Also, shift-click is supposed to open the link in a new window so that needs to be handled as well.
On Thu, Oct 18, 2012 at 10:30 AM, Val Markovic val@markovic.io wrote:
Tested and confirmed as a problem on Ubuntu 12.04 and Mac OS X Mountain Lion. I've already listed the browser versions (basically Chrome latest and older). Note that on Mac, the shortcut is Command-Click, not Control-Click, so the patch doesn't fix the problem on Macs (just confirmed).
Just tested on Safari Version 6.0.1 (8536.26.14), Mac OS X Mountain Lion and Command-Click on a link does not open in a new tab there either (same with Chrome).
On Thu, Oct 18, 2012 at 3:01 AM, Oliver Nightingale < notifications@github.com> wrote:
@Valloric https://github.com/Valloric what os are you using?
If this is still an issue then I'll take a look at getting this merged in quickly, would appreciate some help testing this the fix out on some windows and linux machines where possible, @manuel-woelkerhttps://github.com/manuel-woelkeris this something you would be able to help with too?
— Reply to this email directly or view it on GitHubhttps://github.com/olivernn/davis.js/pull/40#issuecomment-9559209.
olivernn
commented
12 years ago Okay, I've put together a fix for this, it is currently on a branch, you can get a build including the fix here.
I've tested it locally on OS X and it seems to be working as expected but would appreciate it being tried out on a few other browser/os combinations. I have a simple test on jsfiddle here and a gist that should be fairly self explanatory.
If all works as expected I will put together a release for this straight away. Thanks again for your help with this bug.
Valloric
commented
12 years ago Oliver, I've tested your fix on:
The fix works in all of these configurations.
On Fri, Oct 19, 2012 at 4:21 AM, Oliver Nightingale < notifications@github.com> wrote:
Okay, I've put together a fix for this, it is currently on a branchhttps://github.com/olivernn/davis.js/tree/modifier-keys, you can get a build including the fix herehttps://raw.github.com/olivernn/davis.js/modifier-keys/davis.js .
I've tested it locally on OS X and it seems to be working as expected but would appreciate it being tried out on a few other browser/os combinations. I have a simple test on jsfiddle here http://jsfiddle.net/vU4ht/ and a gist https://gist.github.com/3917686 that should be fairly self explanatory.
If all works as expected I will put together a release for this straight away. Thanks again for your help with this bug.
— Reply to this email directly or view it on GitHubhttps://github.com/olivernn/davis.js/pull/40#issuecomment-9597757.
Valloric
commented
12 years ago There was some mention of a release being put together "straight away". Is that still the plan? :)
olivernn
commented
12 years ago I've just pushed a new release with this fix in it.
Capturing all clicks regardless of modifier keys breaks basic expected browser behavior, specifically allowing a new tab to be opened. Middle button mouse button still works as expected. This patch causes the davis listener to ignore ctrl+clicks on links, thereby restoring expected behavior.
Note: I am not sure if that's the correct modifier key for Mac OS. I'd appreciate if somebody with access to a Mac could verify that.