1597061 - In standalone mail window F6 generates error message in console

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Ian Neal]

STR
1/ Open Thunderbird
2/ Open message in a standalone window
3/ Press F6

Expected result
1/ No error message in error console

Actual result
1/ Following message in error console:
ReferenceError: SwitchPaneFocus is not defined messageWindow.xul:1:1

SwitchPaneFocus function is in mail3PaneWindowCommands.js which is not loaded by messageWindow.xul whereas F6 key is defined in mainKeySet.inc.xul which is loaded by messageWindow.xul
Either F6 key definition should be moved to a location not loaded in messageWindow.xul or the function needs moving to somewhere that is loaded by messageWindow.xul. As far as I can see F6 serves no function in a standalone message window so I would say the former is a better option.

Comment 1

[Jorg K (CEST = GMT+2)]

Khushil, is this something you could look into?

Comment 2

[Khushil Mistry [:khushil324]]

Attachment: Bug-1597061_move-F6-keys-mainKeySet.patch

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9109897 [details] [diff] [review]
Bug-1597061_move-F6-keys-mainKeySet.patch

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

::: mail/base/content/hiddenWindow.xul
@@ +74,5 @@
>  </stringbundleset>
>  
>    <!-- keys are appended from the overlay -->
>  <keyset id="mailKeys">
> +  <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);" modifiers="control,shift"/>

I wouldn't think the keys are needed in the hidden window (which is a mac thing). If you don't have the 3pane, there is no pane to switch focus on.

::: mail/base/content/messenger.xul
@@ +230,5 @@
>  <keyset id="mailKeys">
> +  <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);" modifiers="control,shift"/>
> +  <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);" modifiers="control"/>
> +  <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);" modifiers="shift"/>
> +  <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);"/>

hmm, looks like we only ever look at shift (or not) - https://searchfox.org/comm-central/rev/9c5877d219fdda7f6ee2ed352973eb779496e272/mail/base/content/mail3PaneWindowCommands.js#1444

Maybe we can do with just the last one of these?

Comment 4

[Ian Neal]

(In reply to Magnus Melin [:mkmelin] from comment #3)

::: mail/base/content/messenger.xul
@@ +230,5 @@

<keyset id="mailKeys">

• <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);" modifiers="control,shift"/>
• <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);" modifiers="control"/>
• <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);" modifiers="shift"/>
• <key keycode="VK_F6" oncommand="SwitchPaneFocus(event);"/>

hmm, looks like we only ever look at shift (or not) -
https://searchfox.org/comm-central/rev/
9c5877d219fdda7f6ee2ed352973eb779496e272/mail/base/content/
mail3PaneWindowCommands.js#1444

Maybe we can do with just the last one of these?
What happens with the first two removed and the user is holding down the control key?
Does that change the behaviour the user gets? I think at the moment we ignore the control key.

Comment 5

[Khushil Mistry [:khushil324]]

(In reply to Ian Neal from comment #4)

What happens with the first two removed and the user is holding down the control key?
Does that change the behaviour the user gets? I think at the moment we ignore the control key.

Yes, we ignore the control key. Behaviour remains the same after removing the first two keys.

Comment 6

[Khushil Mistry [:khushil324]]

Attachment: Bug-1597061_move-F6-keys-mainKeySet-1.patch

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9109923 [details] [diff] [review]
Bug-1597061_move-F6-keys-mainKeySet-1.patch

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

Doesn't work correctly now with control pressed, so it seems we do need all that we had. r=mkmelin with them added

Comment 8

[Khushil Mistry [:khushil324]]

Attachment: Bug-1597061_move-F6-keys-mainKeySet-2.patch

Comment 9

[Ian Neal]

The same thing also happens with Control and Control+Shift TAB. Another patch on here or a new bug?

Comment 10

[Khushil Mistry [:khushil324]]

Let me do it this patch only. We just need this shift this lines in messenger.xul: https://searchfox.org/comm-central/source/mail/base/content/mainKeySet.inc.xul#142-147

Comment 11

[Khushil Mistry [:khushil324]]

Attachment: Bug-1597061_move-F6-keys-mainKeySet-3.patch - Fix commit message before landing!! [see comment #12]

Comment 12

[Jorg K (CEST = GMT+2)]

Comment on attachment 9110024 [details] [diff] [review]
Bug-1597061_move-F6-keys-mainKeySet-3.patch - Fix commit message before landing!! [see comment #12]

Geoff, in case you land this later, can we please fix the grammar in the commit message:
Bug 1597061 - Fixed in standalone mail window F6 generates error message in console. r=mkmelin

How about:
Fixed error message in console when using F6 in stand-alone mail window

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/afee24ca2f11
Fix error message in console when using F6 in stand-alone mail window. r=mkmelin