Bug Tracker

Ticket #3368 (closed enhancement: fixed)

Opened 6 years ago

Last modified 2 years ago

event.metaKey should be assigned to event.ctrlKey on Non-Mac only

Reported by: tzury Owned by: dmethvin
Priority: low Milestone: 1.7
Component: event Version: 1.4.4
Keywords: needsreview,1.7-discuss Cc:
Blocking: Blocked by:

Description (last modified by ajpiano) (diff)

at jquery.js | version 1.2.6 | line 2150

code:

Add metaKey to non-Mac browsers (use ctrl for PC's and Meta for Macs)

if ( !event.metaKey && event.ctrlKey )

event.metaKey = event.ctrlKey;

This is causing a buggy behavior in the following scenario: A user wish to bind ctrl+x to function-A and command+x to function-B When ctrl+x were pressed, event.metaKey is true as well as event.ctrlKey so function-B is called even though only function-A should be triggered.

The the would obviously be to add additional check for Mac or non Mac operating system, something like:

if ( !event.metaKey && event.ctrlKey && !(/Mac OS/.test(navigator.userAgent))

event.metaKey = event.ctrlKey;

Change History

comment:1 Changed 6 years ago by flesler

  • Owner changed from brandon to flesler
  • Status changed from new to assigned

comment:2 Changed 5 years ago by yehuda

  • Status changed from assigned to closed
  • Resolution set to invalid

The purpose of event.metaKey is to provide a way of binding functionality to "user's native meta key" + "other key". For instance, meta+b could be used to trigger a "bold" mode. On OSX, this would be command+b, but on Windows, it would be ctrl-b.

comment:3 Changed 4 years ago by babyman

  • Status changed from closed to reopened
  • Resolution invalid deleted

Leaving all philosophy aside the Apple keyboard has a command (meta) key AND a control key. It is possible on a Mac to press the Control key WITHOUT pressing the Command key.

Therefore on a Mac meta != control it is simply not true regardless "the purpose of event.meta".

Even the author of the code indicated in their comment that this was intended for PC's only, they simple forgot to filter out Macs:

Add metaKey to non-Mac browsers (use ctrl for PC's and Meta for Macs)

comment:4 Changed 4 years ago by babyman

This seems like it might be a pretty decent workaround for those that are experiencing this issue, instead of:

var cmd = event.metaKey;

use:

var cmd = event.originalEvent.metaKey ? event.originalEvent.metaKey : false;

This means that cmd will be true only when the original event metaKey value was true, in all other cases it will be false even if the control key has been pressed (which seems like reasonable behaviour).

I'm new at this so feedback welcome ;)

comment:5 Changed 4 years ago by snover

  • Keywords needsreview added
  • Priority changed from major to low
  • Version changed from 1.2.6 to 1.4.4
  • Status changed from reopened to open
  • Milestone 1.3 deleted

I think it would make sense to create a separate cmdKey property so people can use metaKey as an “either ctrl or cmd depending upon platform” key, and still retrieve the value of the command key explicitly if they need to.

comment:6 Changed 3 years ago by jquery@…

Well this three year old bug is a mess. Clearly it should be possible to tell the difference between a control and meta key, since the browser can trap any key it wants.

Hopefully the original event is unadulterated at this point so we can do the right thing.

comment:7 Changed 3 years ago by dmethvin

Yes, event.originalEvent is there if you would like to read its tea leaves in a different way. I like snover's idea of a separate cmdKey property but given that there is a lot of code out there using the current properties I don't know if it would make things better or worse.

comment:8 Changed 3 years ago by timmywil

So meta key here refers to the command key? The meta key as I understand it can be changed to alt/option on a Mac and then there is the start button key on pc keyboards. So I guess my question is can we guarantee consistency?

comment:9 Changed 3 years ago by john

  • Type changed from bug to enhancement
  • Milestone set to 1.next

I'm going to make this an enhancement and we can look at adding a new cmdKey in 1.7 (while removing the metaKey hack).

comment:10 Changed 3 years ago by john

  • Keywords needsreview,1.7-discuss added; needsreview removed

Nominating ticket for 1.7 discussion.

comment:11 Changed 3 years ago by dmethvin

+0, I do not know enough to swing on this, but don't think existing code should be broken by a solution.

comment:12 Changed 3 years ago by rwaldron

  • Description modified (diff)

+1,

comment:13 Changed 3 years ago by jaubourg

+0, Seems booby trapped to me

comment:14 Changed 3 years ago by ajpiano

+1, I like the adding of a cmdKey - let people actually test for what they're trying to test for.

comment:15 Changed 3 years ago by timmywil

+0, metaKey works correctly as a proper indicator of pressing command (and it is still possible to differentiate between control and meta). This may be more a problem with the ctrlKey logic.  http://jsfiddle.net/timmywil/wWrAR/

comment:16 Changed 3 years ago by addyosmani

  • Description modified (diff)

+1,

comment:17 Changed 3 years ago by danheberden

  • Description modified (diff)

+1

comment:18 Changed 3 years ago by john

+1

comment:19 Changed 3 years ago by scott.gonzalez

+1

comment:20 Changed 3 years ago by addyosmani

+1

comment:21 Changed 3 years ago by jzaefferer

+1

comment:22 Changed 3 years ago by ajpiano

  • Description modified (diff)
  • Milestone changed from 1.next to 1.7

comment:23 Changed 3 years ago by dmethvin

  • Owner changed from flesler to dmethvin
  • Status changed from open to assigned

Let's see if we can get this closed for 1.7.

On Mac, this code should never be executed I assume; I don't have a Mac to test on. So let's move on to Windows.

On Firefox 6, Chrome 13, Opera 11.5, and IE9, metaKey and ctrlKey move in unison and both are Boolean, so the code isn't needed.

On IE6, IE7 and IE8, metaKey is undefined when the Ctrl key is pressed alone but true when pressed in unison with another key. I suspect this may be why the patch was put in originally. So if we check for typeof(e.metaKey==="undefined") rather than falsey it should handle this case.

comment:24 Changed 3 years ago by dmethvin

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed along with #8789 refactor.

Note: See TracTickets for help on using tickets.