Closed Bug 1327845 Opened 5 years ago Closed 8 months ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: arni2033, Unassigned)

References

Details

>>>   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)).
No longer blocks: 1277113
Component: Untriaged → DOM: Events
Product: Firefox → Core
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.
Hey Stone, could you please help take look at this issue, and suggest the next step for this, such as priority? Thanks!
Flags: needinfo?(sshih)
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.
Flags: needinfo?(sshih)
(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.
(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.
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
Priority: -- → P3
(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
Flags: needinfo?(masayuki)
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.
Flags: needinfo?(masayuki)
(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?
Flags: needinfo?(masayuki)
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.
Flags: needinfo?(masayuki)

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.

Severity: normal → S4
Priority: P3 → P5

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

Status: NEW → RESOLVED
Closed: 8 months ago
Depends on: 1496288
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.