761961 - Typing 8 when a message is selected produces an error in the console

Status: RESOLVED FIXED

Description

[Ludovic Hirlimann [:Usul]]

Select a message , press 8

in the error console you get the following error

Timestamp: 6/6/12 11:13:48 AM
Error: An error occurred executing the cmd_tag8 command: TypeError: tagArray[keyNumber - 1] is undefined
Source File: chrome://global/content/globalOverlay.js
Line: 79

Comment 1

[:aceman]

Actually it adds a tag for me, that is in the context menu under Tags and has a accesskey of 8.
This probably just needs a check if there is a tag defined under that number. I'll try it.

Comment 2

[:aceman]

Note to me:
http://mxr.mozilla.org/comm-central/source/mail/base/content/mailWindowOverlay.js#566

if (tagArray.length < keyNumber)
  return;

Comment 3

[:aceman]

Created attachment 631091 [details] [diff] [review]
patch

Comment 4

[Mike Conley (:mconley)]

Comment on attachment 631091 [details] [diff] [review]
patch

Review of attachment 631091 [details] [diff] [review]:
-----------------------------------------------------------------

Just one super tiny nit. With that fixed, r=me.

::: mail/base/content/mailWindowOverlay.js
@@ +562,5 @@
>    if (!msgHdr)
>      return;
>  
>    let tagArray = MailServices.tags.getAllTags({});
> +  if (tagArray.length < keyNumber)

Sorry for being super super picky, but I think I'd prefer

if (keyNumber > tagArray.length)

I know they're semantically equivalent, but I think it reads easier based on what's expected; "the keynumber the user pressed was out of range" as opposed to "the expected range is outside of the keynumber".

Comment 5

[:aceman]

Created attachment 632386 [details] [diff] [review]
patch v2

What a nit ;)

Comment 6

[:aceman]

Usul, try to verify once this lands.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/ac54b600e9ce