r109808 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r109807‎ | r109808 | r109809 >
Date:05:57, 23 January 2012
Author:santhosh
Status:ok (Comments)
Tags:todo 
Comment:
Fix Bug 33875. Control+ minus key giving keycode of 'm'
Modified paths:
  • /trunk/extensions/Narayam/resources/ext.narayam.core.js (modified) (history)

Diff [purge]

Index: trunk/extensions/Narayam/resources/ext.narayam.core.js
——@@ -116,7 +116,7 @@
117117 return e.altKey == shortcutKey.altKey &&
118118 e.ctrlKey == shortcutKey.ctrlKey &&
119119 e.shiftKey == shortcutKey.shiftKey &&
120 - String.fromCharCode( e.which ).toLowerCase() == shortcutKey.key.toLowerCase();
 120+ String.fromCharCode( e.which ) == shortcutKey.key;
121121 }
122122
123123 /**
——@@ -129,7 +129,7 @@
130130 ctrlKey: true,
131131 shiftKey: false,
132132 cmdKey: false,
133 - key: 'm'
 133+ key: 'M'
134134 };
135135 // Browser sniffing to determine the available shortcutKey
136136 // Refer: mediawiki.util.js and en.wikipedia.org/wiki/Access_key
——@@ -139,7 +139,7 @@
140140 if ( !( profile.platform == 'win' && profile.name == 'safari' ) &&
141141 ( profile.name == 'safari'|| profile.platform == 'mac' || profile.name == 'konqueror' )
142142 && !( profile.name == 'opera' || profile.name == 'chrome' ) ) {
143 - defaultShortcut.key = 'g';
 143+ defaultShortcut.key = 'G';
144144 }
145145 // For Opera in OSX, shortcut is control+command+m.
146146 if ( profile.name == 'opera' && profile.platform == 'mac' ) {

Comments

#Comment by Santhosh.thottingal (talk | contribs)   14:11, 23 January 2012

some more explanation: Keycode of - is 109, but String.fromCharCode(109 ) gives 'm'. Keycode we get when we press control+m is 77. String.fromCharCode(77 ) gives 'M'. Doing a lowercase conversion on both make - key work as control+m. So I am avoiding that lowercase conversion.

#Comment by Hashar (talk | contribs)   14:01, 14 February 2012

Maybe you should only handle keycodes instead? "key: 'M'" is misleading, one would expect it to be the M key on your keyboard instead of minus.

#Comment by Trevor Parscal (WMF) (talk | contribs)   19:02, 14 February 2012

This is very confusing to read because you are mixing concepts here, char codes and key codes.

  • Char codes are mappable ASCII and only come through on keypress events.
  • Key codes are partially mappable (pretty much only lowercase english letters) to ASCII and come through on keydown and keyup events.

People get them confused a lot.

When I trace the execution back, the function fired from a keydown handler, so these are key codes. You should use a number and comment what you mean by the number. Converting a key code to a string using String.fromCharCode( e.which ) is insane.

Status & tagging log