1327845 - Firefox fires 'keypress' event when I cancel drag and drop of file on https://vk.com

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P5)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_1:
1. Open url   data::,<html%20onkeypress="console.log(arguments[0])"><a%20href="http://a.b">Link_1</a>
2. Click on the page
3. Drag some file from hard drive to the center of the page
4. Press Escape key to cancel drag-n-drop

AR:  Page receives 'keypress' event
ER:  Page should not receive 'keypress' event

Explanation:  Page shouldn't receive 'keypress', because
1) Escape is already reserved for canceling drag-n-drop (on system level). Firing this event
   will lead to 2 actions at the same time: canceling drag-n-drop and some action on the page
2) Pressing Escape during normal dnd, when I drag identity block or link doesn't cause 'keypress'
3) I guess it's possible for a page to ignore Escape by monitoring 'dragenter' and 'dragleave',
   but still, usually page expects that it's handled on the browser side.



STR_2:  (originally encountered on https://vk.com/)
1. Log in on https://vk.com (russian social network, facebook clone)
2. Write a message to somebody, go to dialog page (url like https://vk.com/im?sel=123456789)
3. Drag some image from hard drive to the text field in the bottom of the page
4. Press Escape key to cancel drag-n-drop ("I suddenly realized that I don't want to send this image")

Notes:
1) Chrome doesn't fire 'keypress' event, just as expected
2) If you think that Firefox behavior is correct, please change component to "Tech evangelism" and
   possibly suggest a way for the site to fix this behavior (see Explanation -> (3)).

Comment 1

[Marcos Caceres [:marcosc]]

Confirmed on Mac in Nighlty 53. It's tricky to reproduce on Mac, you actually need to get the window focused by going through the application bar. However, once there, indeed the key events are sent.

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hey Stone, could you please help take look at this issue, and suggest the next step for this, such as priority? Thanks!

Comment 3

[Ming-Chou Shih [:stone]]

It's not a common use case but we do send keydown to the content. So does Edge. For some websites such as https://vk.com, users might be surprised at this behavior. I'd suggest put it as P3 and maybe discuss with UX team about the behavior. 

Chrome doesn't send the modifier key events to content alone, which is more friendly to the users (even it may be a bug of the website) from my point of view.

Comment 4

[Ming-Chou Shih [:stone]]

(In reply to Ming-Chou Shih [:stone] from comment #3)
After more tests, Chrome sent keydown of function keys to content and sent keydown, keypress of alphanumeric keys.

Comment 5

[Ming-Chou Shih [:stone]]

(In reply to Ming-Chou Shih [:stone] from comment #3)
> Chrome doesn't send the modifier key events to content alone, which is more
> friendly to the users (even it may be a bug of the website) from my point of
> view.
Correct my observations, Chrome do send keydown and keyup events but not keypress events.

Comment 6

[Ming-Chou Shih [:stone]]

According to the description in UI events spec [1], we should only dispatch keypress for the keys that normally produces a character value.

[1] https://w3c.github.io/uievents/#event-type-keypress

Comment 7

[Ming-Chou Shih [:stone]]

(In reply to Ming-Chou Shih [:stone] from comment #6)
> According to the description in UI events spec [1], we should only dispatch
> keypress for the keys that normally produces a character value.
> 
> [1] https://w3c.github.io/uievents/#event-type-keypress

Hi Masayuki,
I had a trial [1] to stop firing keypress for those keys that don't generate a character value and found that we have some implementations relied on it. Maybe you have some thoughts or plans about keypress event for different keys? 

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=5489b3b8cddd7c4b0198ee7055bd7fd17b6b99ed

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

See bug 354358, it's very long standing bug and really risky to fix it (especially for chrome because chrome scripts and shortcut keys are handled at keypress). Even if the key is a printable key, when it doesn't cause any character, like Ctrl+A, we shouldn't fire keypress. So, checking mKeyNameIndex is not perfect solution. (|mKeyNameIndex != KEY_NAME_INDEX_USE_STRING || mKeyValue.IsEmpty()| means we shouldn't fire keypress, though.)

But I guess that keydown event should be handled to cancel drag and it should call peventDefault().  Then, keypress event won't be fired in this case.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

Oops, bug 968056 is the correct bug#.

Comment 10

[Ming-Chou Shih [:stone]]

(In reply to Masayuki Nakano [:masayuki] from comment #8)
> See bug 354358, it's very long standing bug and really risky to fix it
> (especially for chrome because chrome scripts and shortcut keys are handled
> at keypress). Even if the key is a printable key, when it doesn't cause any
> character, like Ctrl+A, we shouldn't fire keypress. So, checking
> mKeyNameIndex is not perfect solution. (|mKeyNameIndex !=
> KEY_NAME_INDEX_USE_STRING || mKeyValue.IsEmpty()| means we shouldn't fire
> keypress, though.)
> 
> But I guess that keydown event should be handled to cancel drag and it
> should call peventDefault().  Then, keypress event won't be fired in this
> case.
Many thanks for your information. In this case, when dragging a file from hard drive to Firefox and press escape key, the drag is canceled by OS and OS will invoke the registered callback. I'm wondering is there anything else we can do in Gecko?

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

Although, I'm not familiar with D&D implementation, sounds like that if it's possible to detect the cancel reason, the callback should mark next key event is consumed or removed from the message queue. Then, if we take the former, we should dispatch keydown event with marking as defaultPrevented.

Comment 12

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=3 years untouched DOM/Storage bug's priority.

If you have reason to believe this is wrong, please write a comment and ni :jstutte.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

This was fixed by bug 1496288 because we stopped firing keypress event for Escape key press.