Gecko decides keyCode from an ASCII character which is produced by the key
by itself or with Shift on active keyboard layout or alternative ASCII capable
keyboard layout if active keyboard layout isn't ASCII capable. However, we've
ignored alternative ASCII capable keyboard layout's character if both the
key itself and with Shift don't produce ASCII alphabet nor ASCII numeral,
i.e., ASCII punctuation characters are not used in alternative ASCII capable
keyboard layout because of avoiding mapping a keyCode value to 2 or more keys.
However, setting 0 to keyCode value makes Firefox unusable with some web
applications which are aware of neither KeyboardEvent.key nor
KeyboardEvent.code. So, even if we map same keyCode value to a key, we should
avoid setting keyCode value to 0 as far as possible.
This patch's approach is, we behave same keyCode value as the alternative ASCII
capable keyCode is selected when computed keyCode value of active keyboard
layout is 0. This means that we will make some language users whose keyboard
layout for their language is not ASCII capable can use global web services
which support US keyboard layout of Firefox since the new keyCode values
are mostly computed with US layout on Windows or actual alternative ASCII
capable keyboard layout on macOS and Linux. In other words, we cannot improve
compatibility with web applications which don't support Firefox by this patch
since our keyCode values are really different from Chrome's. So, unfortunately,
if we'd use exactly same keyCode computation as Chromium, we'd break
compatibility with existing web applications which are aware of Firefox since
it's necessary to check UA name or something before using keyCode values.
Note that the most important difference between Windows and the others is,
such keyCode value is computed with alternative ASCII capable keyboard
layout on macOS and Linux but only on Windows, it's computed with OEM virtual
keycode. This means that only on Windows, the keyCode value may be different
from actual alternative ASCII capable keyboard layout's keyCode.
MozReview-Commit-ID: As289r9wp6i
This does not remove the directives in widget/ContentCache.cpp as those crash
annotations should be temporary and will be removed once bug 1405832 is fixed.
MozReview-Commit-ID: F0STyYDx8F4
inline functions are supposed to be declared in the header file, so the compiler can inline them.
gcc complains about this, but clang/msvc apparently do not.
We also needed to move the DeadKey struct and class into the header, as the function calls a
method on the DeadKey class, and you can't do that on a forward declared class.
MozReview-Commit-ID: 8NxP59AXuZi
The intention of these macros here is merely to log a warning and enable
a developer to notice the inconsistency. Pass the result to Unused.
MozReview-Commit-ID: 9IOuwQ3InVm
(Path is actually r=froydnj.)
Bug 1400459 devirtualized nsIAtom so that it is no longer a subclass of
nsISupports. This means that nsAtom is now a better name for it than nsIAtom.
MozReview-Commit-ID: 91U22X2NydP
This patch merges nsAtom into nsIAtom. For the moment, both names can be used
interchangeably due to a typedef. The patch also devirtualizes nsIAtom, by
making it not inherit from nsISupports, removing NS_DECL_NSIATOM, and dropping
the use of NS_IMETHOD_. It also removes nsIAtom's IIDs.
These changes trigger knock-on changes throughout the codebase, changing the
types of lots of things as follows.
- nsCOMPtr<nsIAtom> --> RefPtr<nsIAtom>
- nsCOMArray<nsIAtom> --> nsTArray<RefPtr<nsIAtom>>
- Count() --> Length()
- ObjectAt() --> ElementAt()
- AppendObject() --> AppendElement()
- RemoveObjectAt() --> RemoveElementAt()
- ns*Hashtable<nsISupportsHashKey, ...> -->
ns*Hashtable<nsRefPtrHashKey<nsIAtom>, ...>
- nsInterfaceHashtable<T, nsIAtom> --> nsRefPtrHashtable<T, nsIAtom>
- This requires adding a Get() method to nsRefPtrHashtable that it lacks but
nsInterfaceHashtable has.
- nsCOMPtr<nsIMutableArray> --> nsTArray<RefPtr<nsIAtom>>
- nsArrayBase::Create() --> nsTArray()
- GetLength() --> Length()
- do_QueryElementAt() --> operator[]
The patch also has some changes to Rust code that manipulates nsIAtom.
MozReview-Commit-ID: DykOl8aEnUJ
Starting from 52, NativeKey::HandleCharMessage() ignores all control characters. However, some keyboard layout utilities may send WM_CHAR message whose wParam is '\r' for emulating pressing Enter key. For supporting such utilities, we should dispatch Enter keypress event when HandleCharMessage() receives such event.
Note that this patch does NOT support a pair of WM_KEYDOWN and WM_CHAR whose wParam is '\r' but the WM_KEYDOWN isn't VK_RETURN. If there is such case, we need to support it too. However, it needs a lot of code changes. So, we shouldn't support it until such bug is filed actually.
MozReview-Commit-ID: CWyvBtLmXgg
Some IME may handle WM_KEYDOWN message before application and may set the keycode value to VK_PROCSSKEY but not do actually. Similarly, IME may handle WM_KEYDOWN message and replace following WM_CHAR messages with different characters.
Therefore, even if WM_KEYDOWN message comes with VK_PROCESSKEY, NativeKey shouldn't stop dispatching keypress events if it detects following printable char messages.
MozReview-Commit-ID: DcC2qgcLDrQ
Ctrl+Space causes WM_CHAR of ' '. On the other native applications, you can input ' ' with this key combination though, we shouldn't allow this because we need to remove Ctrl and Alt modifier state at dispatching keypress event for the limitation of TextEditor but this is important key combination for custom shortcut keys.
So, when Ctrl or Alt key is pressed but it doesn't change the inputting character, i.e., the character can be inputted without Ctrl or Alt, we shouldn't remove those modifier state from eKeyPress event.
MozReview-Commit-ID: 7omLvNdQWzW
In PresShell, eContextMenu event is marked as dispatched only in chrome when its shiftKey state is true. However, Shift+F10 causes the context menu, it should not be marked as so because this is standard shortcut key to open context menu on Windows.
This patch consumes Shift key state if previous key message is WM_SYSKEYDOWN of F10 before dispatching eContextMenu event. So, user cannot block to prevent its default at opening context menu with Shift+F10, we should discuss this later.
MozReview-Commit-ID: 1P9LpeJoQof
According to crash reports, we may find WM_CHAR whose wParam is 0 and scancode is 0xFF with a call of PeekMessage(PM_NOREMOVE) but we'll remove usual char message with a call of PeekMessage(PM_REMOVE).
In such case, we should ignore the found odd message and take the usual char message which was removed from the queue actually.
MozReview-Commit-ID: Gw8LvCXxul
Currently, we use alias NS_VK_* for WidgetKeyboardEvent::mKeyCode. Similarly, we should create alias enum for nsIDOMKeyEvent::DOM_KEY_LOCATION_*. Then, we can reduce the length and avoid to include nsIDOMKeyEvent in some cpp files.
MozReview-Commit-ID: 5cs4zQ061Lc
I think that when PeekMessage(PM_REMOVE) failed to remove a char message but next key message is still a char message, it may be possible that the odd keyboard layout or utility hook only PeekMessage(PM_NOREMOVE) and GetMessage(). If so, we can explain what occurs in this case.
I'm still not sure this fixes the case of bug 1336322 comment 0, but we should try to do this because I don't have better idea.
MozReview-Commit-ID: CxoO24n167t
This patch depends on bug 1336080.
When PeekMessage() fails to remove found char message, NativeKey::GetFollowingCharMessage() tries to check next key message in the queue again. Then, when next key message becomes non-char message, such as WM_KEYDOWN or WM_KEYUP, the char message must be removed by odd keyboard layout or something. Similarly, when next key message is a char message but it's caused by different key, the found char message must be removed by one of them too.
So, in these cases, NativeKey::GetFollowingCharMessage() should treat the key operation is already handled or canceled by the odd keyboard layout or somebody else. Additionally, in the latter case, following char message should be handled as orphan char message(s) as usual.
MozReview-Commit-ID: 8ahs8I0HUQ2
When NativeKey::GetFollowingCharMessage() tries to remove a char message from the queue, the message might be changed by odd keyboard layout or something. In such case, if the new char message is also caused by same physical key, the char message must be overwritten. Then, we should take the new char message instead.
Note that this patch saves original found char message into kFoundCharMsg and it's logged by each points for indicating if this case has occurred.
MozReview-Commit-ID: HAduq8sfwFt
NativeKey::GetFollowingCharMessage() may remove a char message which is different from previously found message in the queue because hacky keyboard layout or utility can overwrite the wParam when it's removed from the queue.
Now, we should assume that newer message, i.e., actually removed from the queue, is the expected message by the user. See bug 1336028 comment 0 for the actual scenarios which are collected by crash reports.
https://bugzilla.mozilla.org/show_bug.cgi?id=1336028#c0
MozReview-Commit-ID: 9ZgukHH1vfi
Currently, NativeKey::GetFollowingCharMessage() tries 5 times to remove found char message from the queue. It was enough when we found this issue at developing Metrofox.
However, this hack is not enough for some odd keyboard layouts because we see some crash reports which gives up to remove a char message from the queue because 5 WM_NULL messages are returned.
For preventing this crash, we should check if there is the message which is trying to remove from the queue when NativeKey receives WM_NULL. Then, when there is no key message in the queue or next key message becomes non-char message,, NativeKey should dispatch consumed keydown event because we can assume that the key operation may have already been handled or canceled. Otherwise, NativeKey should retry to remove the message again (until 50 times!, it's just enough big magic number, there is no concrete reason).
MozReview-Commit-ID: 1c6Y4OoQdrP
There are still a lot of crash due to failing to get following WM_CHAR message. Almost half of them are, we found a WM_CHAR message but it's gone at removing the message from the queue.
Although, we're still not sure what happen actually. It could be possible if somebody hooks PeekMessage() or something. Then, we can assume that the found message isn't necessary for the user because it must be removed by somebody when it became unnecessary or is handled.
This patch mark a new bool flag, mCharMessageHasGone, as true in such case. Then, NativeKey will dispatch keypress events without following char messages and mark the event as consumed.
MozReview-Commit-ID: mporX1sihC
When NativeKey crashes by itself, it means that we detect an impossible situation in usual environment. In such case, active 3rd party's keyboard layout or something other utility may hook API and returns odd result to us.
For investigating the cause and deciding what we should do in such cases, we should collect active keyboard layout name via crash reports.
MozReview-Commit-ID: HYRj24GwDHZ
I did my best to remove as much stuff as possible in this patch. The starting
point was to remove all the IsVistaOrLater() and IsWin7OrLater() calls, but I
also grepped for various strings and found some other removable stuff that way.
I may have still missed some things.
Notable things done by this patch.
- It removes numerous blocklist entries.
- It removes CanComputeVirtualKeyCodeFromScanCode(), because it's always true
now.
- It removes ShowXP{Folder,File}Picker(), even though these were available as
fallbacks on Vista+. The "when platform is built without the longhorn SDK"
condition in the comment above nsFilePicker::ShowXPFolderPicker() sounds like
it won't ever happen any more.
- It removes the config.trim_on_minimize preference. This requires adding a
bool sHaveInitializedPrefs variable; previously the lack of pref
initialization was indicated by the tri-state sTrimOnMinimize variable having
the value 2.
Notable things *not* done by this patch.
- ClearThemeRegion() still exists. The comment suggests that it is XP/Vista
only, but the code suggests otherwise. jimm thinks the comment is wrong.
- The comment in WinWakeLockListener::Callback() suggests that the StartTimer()
call is no longer needed to block the screen saver. I'm uncertain about this
and so I think it's best left to a follow-up.
When mWidget was already destroyed, anybody shouldn't dispatch WidgetEvent on it. Therefore, NativeKey::InitKeyEvent() has MOZ_CRASH() for detecting such dangerous bug and some users hit it.
Each message handler of NativeKey should check if mWidget has already gone.
Ideally, nsWindow shouldn't create NativeKey and try to handle the message with it. However, using NativeKey's message handlers can put some information to the log. Therefore, this patch doesn't touch nsWindow.
MozReview-Commit-ID: 4k5VfaKHPgG
Currently, KeyboardLayout doesn't support chained dead keys because probably, the initial developer didn't expect there are such keyboard layout. Additionally, if we'd try to handle them with KeyboardLayout, it'd need to create too big and too complicated table at loading such keyboard layout. It's really nightmare. Therefore, this patch takes different approach.
Currently, when WM_(SYS)KEYDOWN is received, KeyboardLayout (and NativeKey) respects following WM_(SYS)CHAR. Similarly, this patch makes KeyboardLayout respect WM_(SYS)DEADCHAR when it handles dead key. If WM_(SYS)KEYDOWN is followed by WM_DEADCHAR, that means that the key press is in a dead key sequence and not finishing the existing dead key sequence. Therefore, when WM_(SYS)KEYDOWN is followed by WM_(SYS)DEADCHAR, KeyboardLayout activates dead key sequence.
For supporting dead key chain, this patch makes KeyboardLayout::mActiveDeadKey and KeyboardLayout::mDeadKeyShiftState arrays. When dead keydown message is received, KeyboardLayout appends an item to each of them. (I.e., when the array is not empty, it's in a dead key sequence.)
When WM_(SYS)KEYUP is received, KeyboardLayout checks if it's in mActiveDeadKey. If it's included in the array, it initializes NativeKey as a dead keyup event.
Otherwise, when non-printable key (probably) is received in a dead key sequence, KeyboardLayout doesn't handle it as a part of the dead key sequence. For example, a modifier key may be pressed for next key. (Even if the keyboard layout maps text input to a non-printable key, we can ignore them because such key's KeyboardEvent.key value should be decided only with the virtual keyboard.)
MozReview-Commit-ID: 9n8B0YYuKCO
Fortunately, UniCharsAndModifiers instances are only in stack. Therefore, we can make it a stack class and use nsAutoString and AutoTArray for not using heap at handling inputs from usual keyboard layouts.
MozReview-Commit-ID: 9ZPbdjGst64
Now, we have an security issue. mCommittedCharsAndModifiers may be initialized with multiple WM_(SYS)CHAR messages. However, if it's generated by odd (or malicious) middleware, mCommittedCharsAndModifiers may be overflown because it has only fixed array. For fixing this issue, first, we should hide the members for making the users not depend on the design of UniCharsAndModifiers.
This patch changes UniCharsAndModifiers to a class and hiding mChars and adding |CharAt() const|.
MozReview-Commit-ID: 5EjrIhmCdE4
This patch creates NativeKey::DispatchKeyPressEventsWithRetrievedCharMessages() for dispatching eKeyPress event with mCommittedCharsAndModifiers when it stores following printable WM_(SYS)CHAR messages.
Using loop for dispatching eKeyPress event for every WM_(SYS)CHAR message is wrong because WidgetKeyboardEvent::mKeyValue is initialized with mCommittedCharsAndModifiers and it causes TextEventDispatcher dispatching multiple eKeyPress events at every call of MaybeDispatchKeypressEvents(). Therefore, if mKeyValue is "^^", eKeyPress event is dispatched 4 times --for the first message, eKeyPress events are fired for each "^" and for the second message, eKeyPress events are fired again for each "^"--. Therefore, when it handles WM_(SYS)KEYDOWN and it causes inputting one or more printable characters, it's the easiest way not to use HandleCharMessage().
The new method calls TextEventDispatcher::MaybeDispatchKeypressEvents() only once and it requests to call the callback method with new argument of MaybeDispatchKeypressEvents() when it needs to dispatch 2 or more eKeyPress events. Then, NativeKey::WillDispatchKeyboardEvent() can set each eKeyPress event to raw information of the message and proper modifier state.
With this change, we can dispatch multiple eKeyPress events with retrieved WM_(SYS)CHAR message information rather than retrieved information from active keyboard layout. Therefore, NeedsToHandleWithoutFollowingCharMessages() doesn't return true even when mCommittedCharsAndModifiers stores two or more characters.
FYI: there is a bug in test_keycodes.xul. That is, Alt+'A' of Greek keyboard layout should cause WM_SYSCHAR with a corresponding Greek character but ASCII characters are specified. Therefore, this patch includes the fix of these bugs
MozReview-Commit-ID: JVm7ZJVug0O
First, mCommittedCharsAndModifiers should be initialized with following char messages because the messages could be different from the information of current keyboard layout.
So, this patch guarantees that mCommittedCharsAndModifiers are same as the user expected when there is one or more WM_CHAR or WM_SYSCHAR messages.
MozReview-Commit-ID: I5Ack0xccoL
This is a simple mistake and blocks following patchs' automated tests.
For example, when Alt+Shift+foo doesn't cause text, this returns false even though it should return true.
MozReview-Commit-ID: 91L33vZhouT
Now, NativeKey::HandleCharMessage() has almost same code, one is for dispatching eKeyPress event for non-printable keys or printable keys when one of Alt or Ctrl key is pressed, the other is for printable keys when Alt or Ctrl key is pressed.
The difference of them is, the former block removes Alt state and Ctrl state for handling AltGr key. When AltGr key is pressed, both Alt and Ctrl state are true. However, EditorBase treas keypress events whose altKey or ctrlKey is true as non-printable key event. Therefore, we need to set these modifier state to false when AltGr key is pressed and the key causes some text.
Note that as far as we know, when a key press with AltGr doesn't cause any characters, WM_CHAR isn't generated. Therefore, we don't need to check with complicated logic if the key event is actually inputting a character.
MozReview-Commit-ID: BRNWfICvkSm
Current shortcut key handling is really difference from what we did before struggling with "key hell". Therefore, remaining hacks for charCode in NativeKey::HandleCharMessage() are not necessary because they are for old code.
MozReview-Commit-ID: 3hvsBOiJ6VV
This patch makes NativeKey::HandleCharMessage() stop dispatching eKeyPress event when the message is inputting a control character. NativeKey::HandleCharMessage() runs following cases:
1. WM_KEYDOWN followed by a WM_CHAR message whose wParam is not a control character causes inputting printable characters.
2. WM_CHAR message is received without WM_KEYDOWN message.
So, when HandleCharMessage() is called for a control character, it's unusual case. E.g., another application sends only WM_CHAR message with a control character.
On the other hand, Gecko is the only browser which dispatches "keypress" event even if pressed printable key doesn't cause inputting any characters. And such "keypress" event is used for shortcut key handling and some default action handling. So, even if we stop dispatching eKeyPress event from HandleCharMessage(), it shouldn't affect most users.
Note that this patch does NOT change the behavior when the user inputs a control character with usual keyboard layout such as Ctrl+A of en-US keyboard layout because DispatchKeyPressEventsWithoutCharMessage() dispatches eKeyPress event in such cases.
This patch also adds a lot of tests with Ctrl or Alt state for detecting regressions.
MozReview-Commit-ID: KUNqTp7RDSc
Currently, Enter and Backspace keys are handled without following WM_(SYS)CHAR message. However, older code needs hack for them for avoiding conflict with Ctrl+J vs. Ctrl+Enter, etc.
So, we can get rid of them now. And when a keydown causes inputting a control character, NativeKey should handle it with keyboard layout (i.e., without following char message(s)).
MozReview-Commit-ID: 6bVuK0BzFbx
Currently, NativeKey::HandleCharMessage() is only a public method and it takes any char message. However, when it's called outside of NativeKey, it should work only with NativeKey::mMsg.
Therefore, we should make current HandleCharMessage() a private method and create new overload method which doesn't take MSG as an argument.
MozReview-Commit-ID: LowV2FUmR3U
NativeKey shouldn't include characters which are provided by WM_SYSCHAR message or WM_DEADCHAR message into mCommittedCharsAndModifiers.
MozReview-Commit-ID: Ax1BmO5wTy0
For consistency with IsPrintableCharMessage(), IsFollowedByNonControlCharMessage() should be renamed to IsFollowedByPrintableCharMessage().
MozReview-Commit-ID: CBJFPO4FZej
Currently, NativeKey::IsFollowedByNonControlCharMessage() returns true only when the first char message is a printable char message. Although, I don't know actual cases coming printable WM_CHAR message after non-printable char message, i.e.,
1. WM_SYSCHAR or WM_DEADCHAR
2. WM_CHAR for a printable character
or
1. WM_CHAR with a non-printable character (a control character)
2. WM_CHAR with a printable character
, we should make it return true because when one or more characters are being inputted, we should ignore non-printable char messages and handle printable char messages in the path handling text input.
MozReview-Commit-ID: 1v7v5mCRFCP
Currently, NativeKey::IsPrintableCharMessage() returns true if given message is WM_CHAR or WM_SYSCHAR. However, WM_SYSCHAR never causes inputting any character and even if WM_CHAR has a control character (i.e., non-printable character), it returns true.
MozReview-Commit-ID: IoU5F06WImz
When key combination is reserved by the system, web apps shouldn't be able to consume the key events. In such case, we've decided that we never expose the key events in web contents (including chrome contents). Therefore, NativeKey stops dispatching keyboard events only for Alt+Space.
However, new code of the fix of bug 1300003 always consumes WM_SYSCHAR which follows WM_SYSKEYDOWN of Alt+Space. This is the cause of bug 1305943.
For fixing this bug, NativeKey should have a helper method, IsReservedBySystem(), and when it returns true, it should do nothing when Handle*Message() is called and should not consume following WM_SYSCHAR messages at handling WM_*KEYDOWN.
Unfortunately, it's impossible to test this regression because nsDOMWindowUtils::SendNativeKeyEvent() may call nsIWidget::SynthesizeNativeKeyEvent() asynchronously. See <https://dxr.mozilla.org/mozilla-central/source/dom/base/nsDOMWindowUtils.cpp#1105-1108>. Therefore, it cannot return if it's consumed.
MozReview-Commit-ID: 9ABh2rYNkFs
Currently, checking if it's in dead key sequence directly refers mActiveDeadKey. However, this isn't clear for most developers. So, let's create a helper method, IsInDeadKeySequence(), and wrap the nonintuitive check.
MozReview-Commit-ID: 7qTer9R8Gfs
For preventing wrapping long lines, old KeyboardLayout::InitNativeKey() uses two variables |virtualKey| and |isKeyDown| which are never modified. However, they may be not clear for some developers where they came from. Additionally, preceding patches reduced a lot of users of them and indent level. Therefore, even if we remove these variables, we don't need additional line breaks in most cases. So, removing them is better for easier to understand.
MozReview-Commit-ID: 680bYVINPAE
This patch creates KeyboardLayout::MaybeInitNativeKeyAsDeadKey() for wrapping dead key handling code in KeyboardLayout::InitNativeKey().
This makes InitNativeKey() code simpler. Now, any developers must be able to understand what InitNativeKey() does easier than before fixing this bug.
MozReview-Commit-ID: C59ESUXeTxU
