1297013 - mozilla::NativeKey should log its behavior

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

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

Currently, we have some regression reports which are difficult to reproduce. Therefore, like IME handlers, NativeKey should log its behavior and make the reporter testable.

Comment 1

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

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

Comment 2

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=26fb48c58b72

Comment 3

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=428a185d4759

Comment 4

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

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

Comment 5

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

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

Comment 6

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c565f868d9e

Comment 7

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

Attachment: Bug 1297013 part.1 Implement ToString(const MSG&) for logging key related message with detail

For logging message, we should have better log text generator for MSG.

Then, we can improve the note of crash reports which is appended in NativeKey::GetFollowingCharMessage().

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

Comment 8

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

Attachment: Bug 1297013 part.2 Implement some helper methods to log constants related to event handling

This patch implements some helper methods to log constants related to event handling.

ToString(KeyNameIndex) and ToString(CodeNameIndex) converts the enum itmes to human readable string.  They use WidgetKeyboardEvent's helper class which returns Unicode text.  Therefore, this need to convert from UTF16 to UTF8.  That's the reason why these methods don't return |const char*|.

GetDOMKeyCodeName(uint32_t) returns DOM keycode name if it's defined.  Otherwise, returns hexadecimal value.  For generating switch-case statement, VirtualKeyCodeList.h shouldn't include ",".  Therefore, this patch removes "," from VirtualKeyCodeList.h and append it at defining NS_DEFINE_VK.  Additionally, the last item of enum and array should not end with ",".  Therefore, this adds dummy last item at each of them.  Finally, some of the keyCode values are shared between 2 keys.  Therefore, it needs to support NS_DISALLOW_SAME_KEYCODE for switch-case generator.  See the comment in the file for more detail.

GetModifiersName(Modifiers) returns all modifier names included in the given value.

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

Comment 9

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

Attachment: Bug 1297013 part.3 Make NativeKey log its behavior

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

Comment 10

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

Comment on attachment 8793148 [details]
Bug 1297013 part.3 Make NativeKey log its behavior

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

Comment 11

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e95482c6151

Comment 12

[Makoto Kato [:m_kato]]

Comment on attachment 8793146 [details]
Bug 1297013 part.1 Implement ToString(const MSG&) for logging key related message with detail

https://reviewboard.mozilla.org/r/79934/#review78722

Comment 13

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

Comment on attachment 8793147 [details]
Bug 1297013 part.2 Implement some helper methods to log constants related to event handling

https://reviewboard.mozilla.org/r/79936/#review78816

Comment 14

[Makoto Kato [:m_kato]]

Comment on attachment 8793148 [details]
Bug 1297013 part.3 Make NativeKey log its behavior

https://reviewboard.mozilla.org/r/79938/#review79318

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1f41988e7a5
part.1 Implement ToString(const MSG&) for logging key related message with detail r=m_kato
https://hg.mozilla.org/integration/autoland/rev/de82c28636a1
part.2 Implement some helper methods to log constants related to event handling r=smaug
https://hg.mozilla.org/integration/autoland/rev/ce34596c89a4
part.3 Make NativeKey log its behavior r=m_kato

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b1f41988e7a5
https://hg.mozilla.org/mozilla-central/rev/de82c28636a1
https://hg.mozilla.org/mozilla-central/rev/ce34596c89a4

Comment 17

[Gerry Chang [:gchang]]

Mark 51 as fix-optional. If it's worth uplift to 51, feel free to nominate it.