Last Comment Bug 786606 - Chat shortcut key Ctrl+Shift+C doesn't do anything - conflicts with keyboard shortcut for Lightning's Calendar tab
: Chat shortcut key Ctrl+Shift+C doesn't do anything - conflicts with keyboard ...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Instant Messaging (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 19.0
Assigned To: Magnus Melin
:
:
Mentors:
Depends on:
Blocks: tb-keyboard-tracker tbkbd-doc-tracker 792074
  Show dependency treegraph
 
Reported: 2012-08-29 03:29 PDT by Wladimir Palant
Modified: 2014-07-28 02:27 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
proposed fix (3.66 KB, patch)
2012-10-02 12:17 PDT, Magnus Melin
bwinton: review+
Details | Diff | Splinter Review
proposed fix (for 17esr, no locailzation changes) (1.60 KB, patch)
2012-10-02 12:26 PDT, Magnus Melin
bwinton: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review
proposed fix, v2 (14.67 KB, patch)
2012-10-14 12:46 PDT, Magnus Melin
mkmelin+mozilla: review+
Details | Diff | Splinter Review

Description Wladimir Palant 2012-08-29 03:29:08 PDT
There is a shortcut key Ctrl+Shift+C displayed for the Go / Chat menu. Pressing that key doesn't do anything however. Looking at http://hg.mozilla.org/comm-central/file/b849a17c4bc1/mail/base/content/mailWindowOverlay.xul#l404 - the key doesn't have any command assigned. It seems that there is oncommand="showChatTab();" missing here.
Comment 1 Thomas D. (currently busy elsewhere; needinfo?me) 2012-08-29 06:56:33 PDT
Wladimir, thank you for reporting and researching this.
Confirming for tb15/winxp.
And yes, the obvious reason is a missing oncommand="..." attribute of the <key>, as referenced in comment 0.

It takes a one-line fix for this one.
Comment 2 Thomas D. (currently busy elsewhere; needinfo?me) 2012-08-29 07:15:52 PDT
Hmmm, according to https://support.mozillamessaging.com/en-US/kb/keyboard-shortcuts, Ctrl+Shift+C is reserved for opening and switching to Lightning's Calendar tab, and it also works that way on my tb15/winxp.

Why do we maintain a keyboard shortcuts list if nobody even bothers to look at it before grabbing new shortcuts? And why was the bug that wanted to add the chat shortcut not added to the keyboard shortcut trackers?
Bug 713979 tb-keyboard-tracker (for pending changes or broken keyboard shortcuts)
Bug 714031 tbkbd-doc-tracker (for tracking in documentation)

Philip won't be amused...
Comment 3 Florian Quèze [:florian] [:flo] 2012-08-29 07:24:08 PDT
(In reply to Thomas D. from comment #2)
> And why was the bug that wanted to add
> the chat shortcut not added to the keyboard shortcut trackers?

Because I didn't know it existed, and none of the reviewers of bug 714733 mentioned it to me.

> Philip won't be amused...

Philip already knows, he noticed during a discussion on IRC a few weeks ago. And yes, that didn't please him, but it was already too late.
Comment 4 Thomas D. (currently busy elsewhere; needinfo?me) 2012-08-29 09:39:56 PDT
1) I suggest the following keyboard shortcuts:

Ctrl+Shift+I for Chat (aka IRC)
Ctrl+Shift+C for Calendar

Imho, calendar is a lot more important than chat, and Lightning has sacrificed a lot of memorizable shortcuts already. It surely deserves to keep one memorizable shortcut for its main page.

2) Alternatively, the other way round:

Ctrl+Shift+I for Calendar
Ctrl+Shift+C for Chat

This might somehow blend in with the existing Lightning Shortcut for New Event (i.e. new Calendar entry), Ctrl+I. So we would have Ctrl+Shift+I for calendar, and Ctrl+I vor New calendar event.

Philip?
Comment 5 Florian Quèze [:florian] [:flo] 2012-08-29 09:48:17 PDT
(In reply to Thomas D. from comment #4)
> 1) I suggest the following keyboard shortcuts:
> 
> Ctrl+Shift+I for Chat (aka IRC)

We expect people to use more Google Talk and Facebook Chat than IRC (IRC is for geeks ;)).
Comment 6 Philipp Kewisch [:Fallen] 2012-08-29 10:52:10 PDT
(In reply to Thomas D. from comment #4)
> 1) I suggest the following keyboard shortcuts:
> 
> Ctrl+Shift+I for Chat (aka IRC)
That is used for DOM Inspector, afaik

> Ctrl+Shift+C for Calendar
I like this, we should do it ;-)

I don't want to give up on this shortcut again, there has been enough pain in the past, its someone else's turn now (sorry Florian ;-)
Comment 7 Thomas D. (currently busy elsewhere; needinfo?me) 2012-08-29 12:39:19 PDT
(In reply to Philipp Kewisch [:Fallen] from comment #6)
> (In reply to Thomas D. from comment #4)
> > 1) I suggest the following keyboard shortcuts:
> > 
> > Ctrl+Shift+I for Chat (aka IRC)
> That is used for DOM Inspector, afaik

Well, DOM Inspector is only for a very small minority of geek users, so if they have to live without shortcut for DOM Inspector, or with a less intuitive shortcut, I don't see a big problem there. Besides, folks who use DOM Inspector are probably capable of tweaking their shortcuts according to their needs, with or without the respective addons to do that.

> > Ctrl+Shift+C for Calendar
> I like this, we should do it ;-)
> I don't want to give up on this shortcut again, there has been enough pain
> in the past, its someone else's turn now (sorry Florian ;-)

(In reply to Florian Quèze [:florian] [:flo] from comment #5)
> (In reply to Thomas D. from comment #4)
> > 1) I suggest the following keyboard shortcuts:
> > Ctrl+Shift+I for Chat (aka IRC)
> We expect people to use more Google Talk and Facebook Chat than IRC (IRC is
> for geeks ;)).

Sure. I'd still maintain that Ctrl+Shift+I might have some mnemonic value for "Chat", users can think of "Internet Chat", "Initiate Chat", "Interaction", "Interconnection" etc. when they try to remember the shortcut.

I didn't check for other free shortcuts, but they are certainly few.
Comment 8 Wladimir Palant 2012-08-29 14:38:55 PDT
(In reply to Philipp Kewisch [:Fallen] from comment #6)
> > Ctrl+Shift+I for Chat (aka IRC)
> That is used for DOM Inspector, afaik

Well, DOM Inspector is only one out of many extensions. Besides, any other shortcut you might choose is likely used by Adblock Plus :)
Comment 9 Magnus Melin 2012-08-31 01:05:42 PDT
(In reply to Philipp Kewisch [:Fallen] from comment #6)
> I don't want to give up on this shortcut again, there has been enough pain
> in the past, its someone else's turn now (sorry Florian ;-)

Agreed.
Comment 10 Thomas D. (currently busy elsewhere; needinfo?me) 2012-08-31 01:09:25 PDT
(In reply to Magnus Melin from comment #9)
> (In reply to Philipp Kewisch [:Fallen] from comment #6)
> > I don't want to give up on this shortcut again, there has been enough pain
> > in the past, its someone else's turn now (sorry Florian ;-)
> 
> Agreed.

+1
Comment 11 foudfou 2012-09-07 05:52:41 PDT
(In reply to Thomas D. from comment #7)
> Sure. I'd still maintain that Ctrl+Shift+I might have some mnemonic value
> for "Chat", users can think of "Internet Chat", "Initiate Chat",
> "Interaction", "Interconnection" etc. when they try to remember the shortcut.

"Instant Messaging" maybe.
Comment 12 Magnus Melin 2012-10-02 12:17:40 PDT
Created attachment 667105 [details] [diff] [review]
proposed fix

Make it Ctrl+Shift+I. (I like the _i_nstant messaging connection).

I changed the localization key to make sure everyone changes it - and it was really mis-capitalized. (I think the convention is for it to be .key or .accesskey, not .accessKey...)

I think the other alternative is to leave it without hotkey - not many complaints so far even if the current advertised hotkey doesn't work...

This patch also makes it actually do something - bug 786606.
Comment 13 Magnus Melin 2012-10-02 12:19:58 PDT
err, commandKey
Comment 14 Magnus Melin 2012-10-02 12:26:59 PDT
Created attachment 667108 [details] [diff] [review]
proposed fix (for 17esr, no locailzation changes)

This should be suitable for tb17,  since it doesn't do anything anyways just remove it.
Comment 15 Florian Quèze [:florian] [:flo] 2012-10-03 08:26:32 PDT
I really dislike this proposed change. Command+shift+I is DOM Inspector. I'm really used to typing that shortcut in Firefox, Thunderbird and Instantbird and apparently I'm not alone as Philipp mentioned this conflict too (comment 6).

(In reply to Thomas D. from comment #7)
> (In reply to Philipp Kewisch [:Fallen] from comment #6)
> > (In reply to Thomas D. from comment #4)
> > > 1) I suggest the following keyboard shortcuts:
> > > 
> > > Ctrl+Shift+I for Chat (aka IRC)
> > That is used for DOM Inspector, afaik
> 
> Well, DOM Inspector is only for a very small minority of geek users,

A small minority who also happens to be a large subset of the developers actually working on the Thunderbird UI. I don't think you want to reduce the efficiency of development work.

> Besides, folks who use
> DOM Inspector are probably capable of tweaking their shortcuts according to
> their needs, with or without the respective addons to do that.

No. Any tweak that has to be repeated each time a new temporary test profile is created is unlikely to be done. It's not a matter of being capable to do it or not.
Comment 16 Florian Quèze [:florian] [:flo] 2012-10-03 08:33:31 PDT
Here is another proposal:
- Fix the Command+shift+C binding so that it opens the Chat tab on a Thunderbird without lightning installed. (I don't see the point of reserving the Command+shift+C binding for users that aren't going to install lightning anyway.)
- Remove the Chat key binding from a XUL overlay in lighting, so that lightning users who are used to having Command+shift+C open a Calandar tab can continue to use that. (I suspect users who do have Lightning installed are more likely to want a key binding for Calendar than for Chat, and could probably live without key binding for the chat tab.)

Philipp, how crazy does this sound? :)
Comment 17 Philipp Kewisch [:Fallen] 2012-10-03 23:02:35 PDT
I personally am just going to write myself a quick and dirty restartless extension that removes the shortcut for chat again, so I can use DOMi. Yes, its too bad for developers, but come on, we know what we are doing :-)

Florian, regarding your proposal, in general I think its a viable option. Of course it might confuse users that are used to Chat and then install Lightning, especially since it makes it look like its Lightning's fault.

Since the number of options are very low, I'd probably go with Cmd+Shift+I.
Comment 18 Blake Winton (:bwinton) (:☕️) 2012-10-11 12:13:57 PDT
Comment on attachment 667105 [details] [diff] [review]
proposed fix

Yeah, Ctrl-Shift-I seems to be the best of a bad set of choices.  Or at least the best choice offered.

>+++ b/mail/base/content/mailWindowOverlay.xul
>@@ -397,17 +397,17 @@
>   <key id="key_collapseAllThreads" key="&collapseAllThreadsCmd.key;" oncommand="goDoCommand('cmd_collapseAllThreads')"/>
>   <key key="&collapseAllThreadsCmd.key;" modifiers="shift"           oncommand="goDoCommand('cmd_collapseAllThreads')"/>
>   <key id="key_nextUnreadThread" key="&nextUnreadThread.key;"        oncommand="goDoCommand('cmd_nextUnreadThread')"/>
>   <key id="key_previousMsg" key="&prevMsgCmd.key;"                   oncommand="goDoCommand('cmd_previousMsg')"/>
>   <key id="key_previousUnreadMsg" key="&prevUnreadMsgCmd.key;"       oncommand="goDoCommand('cmd_previousUnreadMsg')"/>
>   <key id="key_archive" key="&archiveMsgCmd.key;"                    oncommand="goDoCommand('cmd_archive')"/>
>   <key id="key_goForward" key="&goForwardCmd.commandKey;"            oncommand="goDoCommand('cmd_goForward')"/>
>   <key id="key_goBack" key="&goBackCmd.commandKey;"                  oncommand="goDoCommand('cmd_goBack')"/>
>-  <key id="key_goChat" key="&goChatCmd.commandKey;"                  modifiers="accel,shift"/>
>+  <key id="key_goChat" key="&goChatCmd.key;"                         oncommand="showChatTab()" modifiers="accel,shift"/>

If at all possible, I think it would be much better if we could make this use the goDoCommand function, so that we can record it with Test Pilot later...

Other than that, it seems fine.  r=me.
Comment 19 Blake Winton (:bwinton) (:☕️) 2012-10-11 12:15:34 PDT
Comment on attachment 667108 [details] [diff] [review]
proposed fix (for 17esr, no locailzation changes)

I'm not sure if this is needed, or if we can take it at all, but r=me, just in case.
Comment 20 Magnus Melin 2012-10-14 12:46:07 PDT
Created attachment 671252 [details] [diff] [review]
proposed fix, v2

Carrying fwd r=bwinton, using goDoCommand instead (and fixing chat in the standalone window which wasn't working)
Comment 21 Ludovic Hirlimann [:Usul] 2012-10-15 01:06:15 PDT
Magnus did you forget to check this in ?
Comment 22 Magnus Melin 2012-10-15 01:46:08 PDT
The tree is closed so i wasn't able to do it yet.
Comment 23 Magnus Melin 2012-10-19 11:29:02 PDT
http://hg.mozilla.org/comm-central/rev/115a57028c21 -> FIXED
Comment 24 Magnus Melin 2012-10-19 11:31:45 PDT
Comment on attachment 667108 [details] [diff] [review]
proposed fix (for 17esr, no locailzation changes)

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

This is just removing a non-working hotkey advertisement from the menu.
Comment 25 Lukas Blakk [:lsblakk] use ?needinfo 2012-10-19 15:41:32 PDT
Comment on attachment 667108 [details] [diff] [review]
proposed fix (for 17esr, no locailzation changes)

Looks like this should be seeking comm-approvals
Comment 27 Onno Ekker [:nONoNonO UTC+1] 2013-11-29 02:16:30 PST
Too bad Ctrl+Shift+I was also used by DOM inspector, but it's about time DOM Inspector changes that key, because also in Firefox it doesn't work anymore: There it seems to toggle the Web Developers Tools, although the shortcut key isn't listed.
Anyway, DOM Inspector needs some maintenance, because the Find node functionality has been broken for quite some time now :-(
Comment 28 Thomas D. (currently busy elsewhere; needinfo?me) 2014-07-28 02:22:10 PDT
Imo this deserves a relnote for TB31, if only to remind the general public that TB also includes a full chat client
Comment 29 Thomas D. (currently busy elsewhere; needinfo?me) 2014-07-28 02:27:32 PDT
(In reply to Thomas D. from comment #28)
> Imo this deserves a relnote for TB31, if only to remind the general public
> that TB also includes a full chat client

Ops, scrap that, I overlooked the target milestone so this landed for TB24 already but was probably never communicated and isn't on our list of keyboard shortcuts yet, so I wonder if we could just relnote this anyway...

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