1409560 - Period Key Is Not Delete

Status: RESOLVED FIXED

(Calendar :: Tasks, defect)

Description

[Mozilla-User]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161213225041

Steps to reproduce:

Thunderbird's Tasks tab was in focus and a task was highlighted.  I pressed the decimal button on the keyboard number pad.  (Num Lock *was* on.)

A second test was setup as above, but I pressed the period key.  (The key with the ">" and "." symbols.)


Actual results:

My task was deleted.


Expected results:

Nothing should have happened.  The period key and decimal keys (when Num Lock is on) are not delete keys!

Comment 1

[Andre Klapper]

Is this problem 100% reproducible?

Someone remind me which non-numlock options are on those keys, please. :)

Comment 2

[Mozilla-User]

(In reply to Andre Klapper from comment #1)
===
> Is this problem 100% reproducible?
===

Yes.  I can delete, at will, a highlighted task by pressing either the decimal key (with Num Lock turned on) or the period key.

===
> Someone remind me which non-numlock options are on those keys, please. :)
===

LOL.  I found this online:  <http://heavens-feel.com/numpad.jpg>

Comment 3

[Lu Wei]

I can confirm this under WindowsXPSP3 TB 52.4 ESR with Lightning 5.4.4.
Thinkpad X61 keyboard, Key with ">" and "." functions as delete when a task is selected in task list view, both in separate tab or side tab, NumLock on and off.

Comment 4

[Stefan Sitter [:ssitter]]

Using Thunderbird 56.0b3 with Lightning 5.8b3 on Windows 10 nothing happens when pressing the decimal key and NumLock is on. With NumLock off the task is deleted - as expected for numeric keypads according to
https://en.wikipedia.org/wiki/Numeric_keypad
https://en.wikipedia.org/wiki/Num_lock#Keys_affected_by_Num_Lock

There are older bug reports like Bug 772439 that state that NumLock is sometimes not working correctly in Thunderbird and Firefox. But the last comment states that the problem was caused by anti-virus software that messed around with the NumLock system settings.

Comment 5

[Mozilla-User]

(In reply to Stefan Sitter from comment #4)
===
> Using Thunderbird 56.0b3 with Lightning 5.8b3 on Windows 10 nothing happens
> when pressing the decimal key and NumLock is on.
===

I presume pressing the period key also causes nothing to happen?

===
> There are older bug reports like Bug 772439 that state that NumLock is
> sometimes not working correctly in Thunderbird and Firefox. But the last
> comment states that the problem was caused by anti-virus software that
> messed around with the NumLock system settings.
===

This is definitely a different problem.  With NumLock on, the numerical keys do nothing -- except the number 2 (which would be the down arrow) -- it jumps to the top of the task list.  When NumLock is turned of, the navigation keys under the numbers function as expected.

But then again, my Fedora Linux 26 machine is not running an anti-virus program.  ;-)

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to mozilla from comment #5)
> > There are older bug reports like Bug 772439 that state that NumLock is
> > sometimes not working correctly in Thunderbird and Firefox. But the last
> > comment states that the problem was caused by anti-virus software that
> > messed around with the NumLock system settings.
> ===
> 
> This is definitely a different problem.  With NumLock on, the numerical keys
> do nothing -- except the number 2 (which would be the down arrow) -- it
> jumps to the top of the task list.  When NumLock is turned of, the
> navigation keys under the numbers function as expected.
> 
> But then again, my Fedora Linux 26 machine is not running an anti-virus
> program.  ;-)

Do the keys work as they should in other applications with numlock on?

Comment 7

[Mozilla-User]

(In reply to Wayne Mery (:wsmwk) from comment #6)
===
> Do the keys work as they should in other applications with numlock on?
===

Sorry for the delay in responding.  Yes, the keys work ask they should in all other applications when NumLock is enabled.  Even in Thunderbird, when I'm composing a message, pressing the number pad keys produces numbers and arithmetic signs as they should when NumLock is enabled.  (And when NumLock is disabled, the keys produce the expected cursor movements.)

Comment 8

[[:MakeMyDay]]

Are you seeing the same with Firefox? If so, can you please provide STR?

Comment 9

[Mozilla-User]

(In reply to [:MakeMyDay] from comment #8)
===
> Are you seeing the same with Firefox? If so, can you please provide STR?
===

The keyboard's number pad behaves like it should in Firefox.  I have only seen this behavior (the number keypad's "." button acting as if the NumLock is disabled -- i.e. functioning as the Delete button -- even though the NumLock *is* enabled) in Thunderbird Lightning.

This should be trivial to reproduce.  Are you not seeing it on your test machine?  (I.e. create a task, highlight the task, press the number pad's "." key while NumLock is turned on.)

Comment 10

[[:MakeMyDay]]

Ok, I was able to reproduce this in the task view with an EN-US and a FR keyboard layout on Win10, while a DE layout on the same platform works as expected, even though also the same key is used for deletion if numlock is off in all that layouts.

In an input field in FF, like this comment textbox here, this works as expected also for EN-US and FR. Based on that, the initial report and comment 3 this is a multi-platform issue for several keyboard layouts.

I'm not sure whether this is a core or calendar bug. For now let's track it for calendar, we need some further investigation. 

n.b.: iIn EN-US and FR this key is assocciated to DOM_VK_DECIMAL, while in DE it is DOM_VK_SEPARATOR.

Comment 11

[[:MakeMyDay]]

Attachment: FixNumlockIssue-V1.diff

Ok, this is limited to the task tree, all other locations where we capture DOM_VK_DELETE are working properly. With the attached patch, I'm no longer able to reproduce the issue. The removal of event.which is not nesseccary for this fix, but as this is deprecated, let's get rid of it.

We should also consider to uplift this for 5.4.6.

Comment 12

[[:MakeMyDay]]

Attachment: FixNumlockIssue-V2.diff

Sorry, i didn't update the patch before uploading.

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8939171 [details] [diff] [review]
FixNumlockIssue-V2.diff

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

I don't really understand this patch, nor why the order of preventDefault/stopPropagation matters. Care to explain?

event.keyCode is also deprecated, can you use event.key instead? Based on the example at https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key it might make sense to check event.defaultPrevented as well, to avoid the double handling.

Comment 14

[[:MakeMyDay]]

Attachment: FixNumlockIssue-V3.diff

You're right, the order of stopPropagation and preventDefault doesn't matter. I was just adopting the pattern from the other occurences in the code base where the issue doesn't occur.

I did some further debugging:

Numpad Del Key, all Keyboard layouts, numlock off:
>event.code: NumpadDecimal
>event.key: Delete
>event.keyCode: 46
>event.which: 0
>event.getModifierState('NumLock'): FALSE

Numpad Del Key, Keyboard layout DE, numlock on:
>event.code: NumpadDecimal
>event.key: ,
>event.keyCode: 0
>event.which: 44
>event.getModifierState('NumLock'): TRUE

Numpad Del Key, Keyboard layout INTL (=EN-US) or FR, numlock on:
>event.code: NumpadDecimal
>event.key: .
>event.keyCode: 0
>event.which: 46
>event.getModifierState('NumLock'): TRUE

The different value for event.which in the DE layout seems odd, maybe a bug in core, but given that this property is deprecated, probably nobody cares anymore.

The attached patch moves as requested from event.keyCode and event.which to event.key for the task view. With this applied, all three keystrokes in the handler (delete, space and retrun) are captured as expected, while the NumpadDecimal key doesn't trigger a deletion anymore if numlock is on. There's no need to check event.defaultPrevented upfront here.

I'll file a follow-up bug to do the remaining conversion in calendar code.

Comment 15

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8939235 [details] [diff] [review]
FixNumlockIssue-V3.diff

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

Thanks, looks good! I'm ok to make the order of stopPropagation/preventDefault consistent if you like, I had just thought this magically fixed the issue and was confused.

Comment 16

[[:MakeMyDay]]

Thanks, let's leave it as is.

Comment 17

[[:MakeMyDay]]

Comment on attachment 8939235 [details] [diff] [review]
FixNumlockIssue-V3.diff

We should uplift this once it has been in a beta. beta approval request is just in case there will be another 58 beta.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c24349d92dd8
avoid deletion in task tree if decimal key on the numpad is pressed with numlock on. r=philipp

Comment 19

[Jorg K (CEST = GMT+2)]

Beta (TB 58, Calendar 6.0):
https://hg.mozilla.org/releases/comm-beta/rev/903e691f07ce

Comment 20

[Jorg K (CEST = GMT+2)]

TB 52.6 ESR, Calendar 5.4.6:
https://hg.mozilla.org/releases/comm-esr52/rev/da5bd978de279a3a6ede0614416f354cae36bbb5

Please create at set target to 5.4.6.