586719 - Use proper KeyEvent constants for Tab Candy code

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect, P2)

Description

[Ian Gilman [:iangilman]]

From our dolske's review of bug 574217:

> >+  _setTabViewFrameKeyHandlers: function() {
> 
> Instead of the various magic numbers in this function, you really ought to be
> using KeyEvent.DOM_VK_BLAH (.DOM_VK_TAB, .DOM_VK_RIGHT, etc).

Comment 1

[Raymond Lee [:raymondlee]]

Attachment: v1 for _setTabViewFrameKeyHandlers

Updated the magic numbers to constants.

Comment 2

[(no longer active)]

Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".

Comment 3

[Ed Lee :Mardak]

There's a few number key codes in browser-tabview.js too

Comment 4

[Raymond Lee [:raymondlee]]

Attachment: v1 for _setBrowserKeyHandlers

* Replaced a number with KeyCode.DOM_VK_*, that's the only number I can use in this part of code based on http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/events/nsIDOMKeyEvent.idl
* Fixed an issue which changing the active group in the reverse order was broken.

Comment 5

[Ian Gilman [:iangilman]]

Comment on attachment 466532 [details] [diff] [review]
v1 for _setBrowserKeyHandlers

Looks good to me.

Comment 6

[Justin Dolske [:Dolske]]

Comment on attachment 466532 [details] [diff] [review]
v1 for _setBrowserKeyHandlers

>           charCode == 160) { // alt + space
...
>           (charCode == 96 || charCode == 126)) {

You could use |"x".charCodeAt(0)|, but I guess it's fine as-is too.


>+++ b/browser/base/content/tabview/groupitems.js	Tue Aug 17 11:01:00 2010 +0800

Please put these changes into a separate bug. Bugs are cheap, and one issue per bug makes Hg history easier to understand.

r+ with that change.

Comment 7

[Raymond Lee [:raymondlee]]

Attachment: v2 for _setBrowserKeyHandlers

r+ Dolske f+ Ian

Comment 8

[Raymond Lee [:raymondlee]]

Attachment: v1 for _setTabViewFrameKeyHandlers [r=dolske a=dolske]

Comment 9

[Raymond Lee [:raymondlee]]

Attachment: v2 for _setBrowserKeyHandlers [r=dolske a=gavin]

Comment 10

[Ed Lee :Mardak]

http://hg.mozilla.org/mozilla-central/rev/bd255a24160a
http://hg.mozilla.org/mozilla-central/rev/17d24407f975