Last Comment Bug 761961 - Typing 8 when a message is selected produces an error in the console
: Typing 8 when a message is selected produces an error in the console
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All All
-- normal (vote)
: Thunderbird 16.0
Assigned To: :aceman
Depends on:
  Show dependency treegraph
Reported: 2012-06-06 02:17 PDT by Ludovic Hirlimann [:Usul]
Modified: 2012-06-12 13:54 PDT (History)
3 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch (980 bytes, patch)
2012-06-07 12:52 PDT, :aceman
mconley: review+
Details | Diff | Splinter Review
patch v2 (990 bytes, patch)
2012-06-12 13:32 PDT, :aceman
no flags Details | Diff | Splinter Review

Description User image Ludovic Hirlimann [:Usul] 2012-06-06 02:17:26 PDT
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 User image :aceman 2012-06-07 01:07:24 PDT
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 User image :aceman 2012-06-07 02:43:28 PDT
Note to me:

if (tagArray.length < keyNumber)
Comment 3 User image :aceman 2012-06-07 12:52:10 PDT
Created attachment 631091 [details] [diff] [review]
Comment 4 User image Mike Conley (:mconley) 2012-06-12 13:18:21 PDT
Comment on attachment 631091 [details] [diff] [review]

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 User image :aceman 2012-06-12 13:32:42 PDT
Created attachment 632386 [details] [diff] [review]
patch v2

What a nit ;)
Comment 6 User image :aceman 2012-06-12 13:33:31 PDT
Usul, try to verify once this lands.
Comment 7 User image Ryan VanderMeulen [:RyanVM] 2012-06-12 13:54:47 PDT

Note You need to log in before you can comment on or make changes to this bug.