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
--HG--
extra : rebase_source : 3b8a2aaeb4c53be1567f7a8d17dfef1af90406dd
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
--HG--
extra : rebase_source : 7387fc89930f9b15d5d01612eb4a7a5726924bcd
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
--HG--
extra : rebase_source : f91e37b0b82a8e8a95444a08f6d3101f14fadc50
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
--HG--
extra : rebase_source : 08ff45c1990ef8f3ed7703c69b2dc22e5f1dd6f5
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
--HG--
extra : rebase_source : 271c61515752cf1ea56cf12c4f7ef6f1a90c9cac
NativeKey shouldn't include characters which are provided by WM_SYSCHAR message or WM_DEADCHAR message into mCommittedCharsAndModifiers.
MozReview-Commit-ID: Ax1BmO5wTy0
--HG--
extra : rebase_source : 5d7d1735a783213f7c6a47cca0cadb1d4c5104c0
For consistency with IsPrintableCharMessage(), IsFollowedByNonControlCharMessage() should be renamed to IsFollowedByPrintableCharMessage().
MozReview-Commit-ID: CBJFPO4FZej
--HG--
extra : rebase_source : f5d677743be35a3bb5fca35e7658e907aa46dbcb
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
--HG--
extra : rebase_source : f82f7b77376450168bad34be031164ecf7338621
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
--HG--
extra : rebase_source : c48f424f3d2960e6e9ceaa6d2fb09952ca634c3f
This guarantees that the runnable will be processed before shutdown leak checking.
MozReview-Commit-ID: B89AcDUWb3y
--HG--
extra : rebase_source : bfbd311fba367c1856cc5d972e6e999ccd4de1ac
Bug 1305498 - 1. Remove NotificationClient task queue; r=sebastian
Not sure why we needed a task queue for NotificationClient actions. The
actions all go through IPC and are non-blocking, so it's perfectly fine
to perform them off of whatever thread we're on.
Bug 1305498 - 2. Integrate NotificationHandler et al into NotificationCllient; r=sebastian
There's no reason to have NotificationHandler, AppNotificationClient,
and ServiceNotificationClient all separate from the base
NotificationClient class. This patch adds the functionality of
those three classes to NotificationClient.
The notifications hash map is changed from a ConcurrentHashMap to a
regular HashMap with synchronization because I think the use case here
doesn't warrant the added performance and overhead of ConcurrentHashMap.
NotificationService is changed to match the new NotificationClient. Now
the only job for NotificationService is to set a notification as
foreground, rather than to manage all notifications like before.
NotificationHandler, AppNotificationClient, and
ServiceNotificationClient will be removed in a later patch.
Bug 1305498 - 3. Set NotificationListener in GeckoApplication; r=sebastian
Set NotificationListener once in GeckoApplication.onCreate, instead of
spreading it out in GeckoApp, BrowserApp, and GeckoService. This is
possible because there's no longer a distinction between
AppNotificationClient and ServiceNotificationClient in the new,
consolidated NotificationClient.
Bug 1305498 - 4. Remove obsolete notification classes; r=sebastian
Remove AppNotificationClient, ServiceNotificationClient, and
NotificationHandler, now that they've all been replaced by the new,
consolidated NotificationClient.
Bug 1305498 - 5. Use NotificationReceiver for web notification callbacks; r=sebastian
Previously, web notification callbacks went to GeckoApp directly, but
that presented some problems such as not being able to implement the
on-close callback, because we don't want to launch GeckoApp when the
notification is closed by swiping. This patch makes us use
NotificationReceiver for callbacks, and let NotificationReceiver launch
GeckoApp if necessary.
Bug 1305498 - 6. Don't keep notification cookie in native code; r=sebastian
Keep the notification cookie a single location (in the notification
intent itself), and simplify the native notification handling code.
Bug 1305498 - 7. Use NotificationReceiver for persistent notifications; r=sebastian
Currently, persistent notification callbacks go through a different code
path, but it'd be more consistent and correct to let persistent
notification callbacks go through NotificationReceiver as well.
This takes care of some housekeeping work that was missing for
persistent notifications, such as deleting the mNotifications entry when
the notification is closed.
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
--HG--
extra : rebase_source : 7416e45babccce3af795dc523f46dc00fdddc6c7
We usually declare class members with following order:
1. public members
2. private members
3. friends
However, current KeyboardLayout is declared with completely opposite order. Let's use usual order for preventing mistakes.
MozReview-Commit-ID: LwOcEUbJzes
--HG--
extra : rebase_source : 828b5d14f1b38f46e4ae5244d15de68c4db6785c
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
--HG--
extra : rebase_source : 6ecb20cc3f5e28cfcf41fb33df8603785288bdda
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
--HG--
extra : rebase_source : 15f4398072ccfe659fc6d2ae95a3f13261efa5af
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
--HG--
extra : rebase_source : c840a0544470ec4081e80c11e050d4dfedef4cad
There is DeactivateDeadKeyState() but not ActivateDeadKeyState(). There should be both of them and hides the dead key state management into them.
MozReview-Commit-ID: JvAPE5f2HVy
--HG--
extra : rebase_source : d7027a1a6b75c6859e79d651d3e9b0136cc09891
VirtualKey::GetCompositeChar() needs index of virtual keycode which may make around the caller messy. So, KeyboardLayout::GetCompositeChar() should wrap it and KeyboardLayout::MaybeInitNativeKeyWithCompositeChar() should use it.
MozReview-Commit-ID: 8KLTJpCFZ8u
--HG--
extra : rebase_source : fd13c009330fd485c8cba30f469fc645b560c623
Similar to VirtualKey::GetUniChars(), VirtualKey::GetNativeUniChars() needs key index. So, it should be wrapped by a new helper method, KeyboardLayout::GetNativeUniCharsAndModifiers(), and KeyboardEvent::InitNativeKey() should use this instead of accessing its member's GetNativeUniChars().
MozReview-Commit-ID: 7M9OlNF698Y
--HG--
extra : rebase_source : e19a5c99014a8f0a0ed7cc3a86fe0c4ee10cebb5
When InitNativeKey() retrieves UniCharsAndModifiers for a key, it needs key index for the given virtual keycode. Therefore, wrapping the code with GetUniCharsAndModifiers() makes InitNativeKey() code simpler since each call specifies the virtual keycode to the method instead of key index.
MozReview-Commit-ID: Azy8chXexaz
--HG--
extra : rebase_source : 98da43e7f542037f952206cd3db731f6cbb097e7
This patch gets rid of |shiftState| from KeyboardLayout::InitNativeKey() and make each caller clearer. This must make other developers understand what modifier state is used at each call.
MozReview-Commit-ID: 6zydP1jkffv
--HG--
extra : rebase_source : ea1d5f49ea02812aa632bdbad871688879f2b401
KeyboardLayout::InitNativeKey() is very messy because it handles a lot of cases without helper methods.
It's important to make it simpler implementation for preventing regressions caused by some patches which are written with misunderstanding. So, let's rewrite the method with helper method.
First, this patch make InitNativeKey() use IsDeadKey() instead of referring the table because calling IsDeadKey() is easier to understand.
MozReview-Commit-ID: DtN9qoh7Gz7
--HG--
extra : rebase_source : 8e769c1c88bfe646940491c111de609dbf53eeec
Make contentDocumentChanged and isContentDocumentDisplayed calls require
the caller to pass in a window object, so that we can get the widget and
GeckoLayerClient from the window object. This way these calls no longer
depend on having a global layer client in AndroidBridge.
Add AndroidCompositorWidget to act as the intermediary between gfx code
and GeckoLayerClient, in place of AndroidBridge. AndroidCompositorWidget
currently inherits from InProcessCompositorWidget, but when Android
eventually supports OOP compositing, it will be made to inherit from
CompositorWidget directly.
Using direction-changed signal, we detect keyboard change for bidi.
When system uses fcitx's IM and ibus's arabic keyboard layout, this signal might fire often when switing layout and gdk_keymap_get_direction might return invalid bidi infromation. But I think that this is rare issue. Most users don't use Firefox Arabic version (it means that bidi.browser.ui = true) with ibus arabic layout and fcitx CJK IM. Since there is no GTK3 API to get current IM module, I cannot find workaround for this.
MozReview-Commit-ID: DL8uUXJFWYz
--HG--
extra : rebase_source : 18b7d5c84f48acd62fa14e21a67b711106fb7d63