1510500 - Use #ifdef in a file to declare shortcut keys in dom/xbl/builtin instead of using ShortcutKeyDefinitions.cpp per platform

Status: RESOLVED FIXED

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

Description

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

Because of the fix of bug 1419091, some shortcut keys (mainly for editor) are declared with array of C++ instead of XUL files. However, for making the review easier in the bug, platform specific shortcut keys are declared in separated ShortcutKeyDefinitions.cpp for each platform. Therefore, when I investigate shortcut keys, it's not unclear which shortcut key won't work in which platforms with synthesized key events, etc.

So, I'd like to combine them into a C++ file and using #ifdefs to enable/disable some of them only on specific platforms.

Comment 1

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

Resetting assignee which I don't work on in this several months.

Comment 2

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

Attachment: Bug 1510500 - part 1: Add GTest for checking no unexpected changes in `ShortcutKeyDefinitions.cpp` r=edgar!

Making the following changes and future changes to the shortcut key definitions
safer, we should have a GTest for checking unexpected changes.

Comment 3

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

Attachment: Bug 1510500 - part 2: Move ShortcutKeyDefinitions.cpp for Windows to XP part r=edgar!

This will have shortcut keys definitions for the other platforms with #ifs.

Depends on D103982

Comment 4

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

Attachment: Bug 1510500 - part 3: Move ShortcutKeyDefinitions.cpp for Linux to the XP ShortcutKeyDefinitions.cpp r=edgar!

Depends on D103983

Comment 5

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

Attachment: Bug 1510500 - part 4: Move ShortcutKeyDefinitions.cpp for Android to the XP ShortcutKeyDefinitions.cpp r=edgar!

Depends on D103984

Comment 6

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

Attachment: Bug 1510500 - part 5: Move ShortcutKeyDefinitions.cpp for Emacs to the XP ShortcutKeyDefinitions.cpp r=edgar!

Depends on D103985

Comment 7

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

Attachment: Bug 1510500 - part 6: Move ShortcutKeyDefinitions.cpp for macOS to the XP ShortcutKeyDefinitions.cpp r=edgar!

Depends on D103986

Comment 8

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

Attachment: Bug 1510500 - part 7: Move all `ShortcutKeyDefinitionsFor*.h` into the XP `ShortcutKeyDefinitions.cpp` r=edgar!

Then, we can delete duplicated entries in some platforms.

Depends on D103987

Comment 9

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

Attachment: Bug 1510500 - part 8: Group arrow key shortcut definitions r=edgar!

For making the definitions easier to read, same definition across platforms
should be merged. For doing it and making each change review easier, we should
same things for each key group.

Depends on D103988

Comment 10

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

Attachment: Bug 1510500 - part 9: Group PageDown/PageUp key shortcut definitions r=edgar!

Depends on D103989

Comment 11

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

Attachment: Bug 1510500 - part 10: Group Home/End key shortcut definitions r=edgar!

Depends on D103990

Comment 12

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

Attachment: Bug 1510500 - part 11: Group Insert key shortcut definitions r=edgar!

Depends on D103991

Comment 13

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

Attachment: Bug 1510500 - part 12: Group Delete/Backspace key shortcut definitions r=edgar!

Depends on D103992

Comment 14

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

Attachment: Bug 1510500 - part 13: Group Accel(-Shift)-C/X/V/Z key shortcut definitions r=edgar!

Depends on D103993

Comment 15

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

Attachment: Bug 1510500 - part 14: Group space key shortcut definitions r=edgar!

Depends on D103994

Comment 16

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

Attachment: Bug 1510500 - part 15: Group the other shortcut definitions r=edgar!

Depends on D103995

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/332db3910c59
part 1: Add GTest for checking no unexpected changes in `ShortcutKeyDefinitions.cpp` r=edgar
https://hg.mozilla.org/integration/autoland/rev/82fb047175b2
part 2: Move ShortcutKeyDefinitions.cpp for Windows to XP part r=edgar
https://hg.mozilla.org/integration/autoland/rev/04ca138667bb
part 3: Move ShortcutKeyDefinitions.cpp for Linux to the XP ShortcutKeyDefinitions.cpp r=edgar
https://hg.mozilla.org/integration/autoland/rev/4da1a785ff65
part 4: Move ShortcutKeyDefinitions.cpp for Android to the XP ShortcutKeyDefinitions.cpp r=edgar
https://hg.mozilla.org/integration/autoland/rev/57910e3df62a
part 5: Move ShortcutKeyDefinitions.cpp for Emacs to the XP ShortcutKeyDefinitions.cpp r=edgar
https://hg.mozilla.org/integration/autoland/rev/f85c65d972a3
part 6: Move ShortcutKeyDefinitions.cpp for macOS to the XP ShortcutKeyDefinitions.cpp r=edgar
https://hg.mozilla.org/integration/autoland/rev/8673c1c954e6
part 7: Move all `ShortcutKeyDefinitionsFor*.h` into the XP `ShortcutKeyDefinitions.cpp` r=edgar
https://hg.mozilla.org/integration/autoland/rev/84e422db7674
part 8: Group arrow key shortcut definitions r=edgar
https://hg.mozilla.org/integration/autoland/rev/0b0a21b506b4
part 9: Group PageDown/PageUp key shortcut definitions r=edgar
https://hg.mozilla.org/integration/autoland/rev/2c28bee095f9
part 10: Group Home/End key shortcut definitions r=edgar
https://hg.mozilla.org/integration/autoland/rev/80eae543a485
part 11: Group Insert key shortcut definitions r=edgar
https://hg.mozilla.org/integration/autoland/rev/d05f359fb25c
part 12: Group Delete/Backspace key shortcut definitions r=edgar
https://hg.mozilla.org/integration/autoland/rev/c5a9cd3aef19
part 13: Group Accel(-Shift)-C/X/V/Z key shortcut definitions r=edgar
https://hg.mozilla.org/integration/autoland/rev/c5cd3697e6d8
part 14: Group space key shortcut definitions r=edgar
https://hg.mozilla.org/integration/autoland/rev/7da855a143d6
part 15: Group the other shortcut definitions r=edgar

Comment 18

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/332db3910c59
https://hg.mozilla.org/mozilla-central/rev/82fb047175b2
https://hg.mozilla.org/mozilla-central/rev/04ca138667bb
https://hg.mozilla.org/mozilla-central/rev/4da1a785ff65
https://hg.mozilla.org/mozilla-central/rev/57910e3df62a
https://hg.mozilla.org/mozilla-central/rev/f85c65d972a3
https://hg.mozilla.org/mozilla-central/rev/8673c1c954e6
https://hg.mozilla.org/mozilla-central/rev/84e422db7674
https://hg.mozilla.org/mozilla-central/rev/0b0a21b506b4
https://hg.mozilla.org/mozilla-central/rev/2c28bee095f9
https://hg.mozilla.org/mozilla-central/rev/80eae543a485
https://hg.mozilla.org/mozilla-central/rev/d05f359fb25c
https://hg.mozilla.org/mozilla-central/rev/c5a9cd3aef19
https://hg.mozilla.org/mozilla-central/rev/c5cd3697e6d8
https://hg.mozilla.org/mozilla-central/rev/7da855a143d6