Closed Bug 1210585 Opened 4 years ago Closed 4 years ago

Convert key events to native calls


(Core :: Widget: Android, defect)

Not set



Tracking Status
firefox44 --- fixed


(Reporter: jchen, Assigned: jchen)


(Blocks 1 open bug)



(6 files)

Convert KEY_EVENT and IME_KEY_EVENT in GeckoEvent to native calls.
Java allows a class field to have the same name as a superclass field,
but when we generate bindings for them, they'll end up with the same C++
name and cause an error. This patch makes the SDK processor filter out
any superclass fields that are hidden by a subclass field with the same
Attachment #8669976 - Flags: review?(snorp)
The code generator uses == and != to compare two instances of Class, but
it really should be using equals because two distinct instances of Class
can refer to the same class type.
Attachment #8669977 - Flags: review?(snorp)
Autogenerate C++ bindings for the SDK class KeyEvent.
Attachment #8669978 - Flags: review?(esawin)
This patch adds a native call to GeckoEditable that will handle key
events instead of using GeckoEvent.KEY_EVENT.
Attachment #8669981 - Flags: review?(snorp)
This patch implements a new key event handler in nsWindow to replace the
previous implementation.

nsWindow::HandleSpecialKey was removed because it's a relic from XUL
Fennec and I believe we no longer need it for native Fennec.
Attachment #8669983 - Flags: review?(esawin)
Attachment #8669981 - Flags: review?(snorp) → review?(esawin)
Attachment #8669976 - Flags: review?(snorp) → review+
Attachment #8669977 - Flags: review?(snorp) → review+
Attachment #8669978 - Flags: review?(esawin) → review+
Comment on attachment 8669981 [details] [diff] [review]
Add native calls for key events (v1)

Review of attachment 8669981 [details] [diff] [review]:

::: mobile/android/base/
@@ +82,5 @@
> +                                   int domPrintableKeyValue, int repeatCount, int flags,
> +                                   boolean isSynthesizedImeKey);
> +
> +    private void onKeyEvent(KeyEvent event, int action, int savedMetaState,
> +                            boolean isSynthesizedImeKey) {

nit: it would be nicer to have an enum for the event type instead of a boolean argument.
Attachment #8669981 - Flags: review?(esawin) → review+
Comment on attachment 8669983 [details] [diff] [review]
Implement new key event handler in nsWindow (v1)

Review of attachment 8669983 [details] [diff] [review]:

::: widget/android/nsWindow.cpp
@@ +208,5 @@
> +    void OnKeyEvent(int32_t action, int32_t keyCode, int32_t scanCode,
> +                    int32_t metaState, int64_t time, int32_t unicodeChar,
> +                    int32_t baseUnicodeChar, int32_t domPrintableKeyValue,
> +                    int32_t repeatCount, int32_t flags,
> +                    bool isSynthesizedImeKey);

Just noticed that we have a wild mix of ("a"-)prefixed and non-prefixed arguments in android/nsWindows.cpp while the other */nsWindows.cpp are consistently using the prefixed style. We should stick to it, too, for consistency.

@@ +434,5 @@
>  {
>      nsBaseWidget::mOnDestroyCalled = true;
> +    if (mNatives) {
> +        // Disassociaye our native object with GeckoView.

Attachment #8669983 - Flags: review?(esawin) → review+
This patch changes the parameter names in IME-related functions in nsWindow to be prefixed.
Attachment #8671148 - Flags: review+
You need to log in before you can comment on or make changes to this bug.