1438133 - Ctrl + Enter should cause keypress event even after fixing bug 968056

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, enhancement, P2)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

On the other browsers, it depends on the platform when Enter key causes keypress event. I think that basically, we shouldn't fire keypress event for Enter key unless it causes breaking line in <textarea> or contenteditable element. (Especially, even Chrome fires keypress event for Enter key with some modifiers, it doesn't modify both editors.)

However, for compatibility, we should keep dispatching keypress event for Ctrl + Enter since Ctrl + Enter causes keypress event on any browsers with any platforms. I believe that we don't need to emulate inconsistency between platforms of Chrome.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=54fc3e3d05d7b9b50d75b241b5aca8fb0ff9b21f

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=38edbc920647851d4ed3dc717df2ba7455a25ca1

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1438133 - Ctrl + Enter should cause keypress event even though the key combination doesn't input any character

Currently, we dispatch keypress event when Enter is pressed without modifiers
or only with Shift key.  However, the other browsers dispatch keypress event
for Ctrl + Enter in any platforms even if it doesn't cause any text input.
So, we should fire keypress event for Ctrl + Enter even in strict keypress
dispatching mode.

Note that with other modifiers, it depends on browser and/or platform.
So, anyway, web developers shouldn't use keypress event to catch
Alt + Enter, Meta + Enter and two or more modifiers + Enter.

Review commit: https://reviewboard.mozilla.org/r/220822/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220822/

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8951524 [details]
Bug 1438133 - Ctrl + Enter should cause keypress event even though the key combination doesn't input any character

https://reviewboard.mozilla.org/r/220822/#review227048

::: widget/TextEvents.h:255
(Diff revision 1)
>                             MODIFIER_META |
>                             MODIFIER_OS));
>    }
>  
> +  /**
> +   * ShouldKeyPressEventBeFiredOnContent() should be called only when the

This is inconsistent with the actual method name.
I think using ShouldKeyPressEventBeFiredOnContent as the method name would be good.

::: widget/TextEvents.h:266
(Diff revision 1)
> +    MOZ_DIAGNOSTIC_ASSERT(mMessage == eKeyPress);
> +    if (IsInputtingText() || IsInputtingLineBreak()) {
> +      return true;
> +    }
> +    // Ctrl + Enter won't cause actual input in our editor.
> +    // However, the other browsers fires keypress event in any platforms.

s/fires/fire/

::: widget/TextEvents.h:268
(Diff revision 1)
> +      return true;
> +    }
> +    // Ctrl + Enter won't cause actual input in our editor.
> +    // However, the other browsers fires keypress event in any platforms.
> +    // So, for compatibility with them, we should fire keypress event for
> +    // Ctrl + Enter too.

Since this is really odd behavior, could you file a spec bug to define this there too.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8951524 [details]
Bug 1438133 - Ctrl + Enter should cause keypress event even though the key combination doesn't input any character

https://reviewboard.mozilla.org/r/220822/#review227048

> Since this is really odd behavior, could you file a spec bug to define this there too.

Filed: https://github.com/w3c/uievents/issues/183

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8951524 [details]
Bug 1438133 - Ctrl + Enter should cause keypress event even though the key combination doesn't input any character

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220822/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb4185c40f3d
Ctrl + Enter should cause keypress event even though the key combination doesn't input any character r=smaug

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/cb4185c40f3d