Closed Bug 1211704 Opened 4 years ago Closed 4 years ago

Convert IME events to native calls

Categories

(Core :: Widget: Android, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

(Blocks 1 open bug)

Details

Attachments

(6 files)

No description provided.
These native methods will replace the IME events used by GeckoEditable.
Attachment #8671149 - Flags: review?(esawin)
Remove IME events from GeckoEvent and use the newly added native calls
in GeckoEditable. The changes are mostly straightforward find & replace.
Attachment #8671150 - Flags: review?(esawin)
Move the IME event handler implementation in nsWindow::OnIMEEvent to
individual native calls in nsWindow::Natives. This patch also moves most
member variables and helper functions related to IME to inside
nsWindow::Natives. This has the benefit of better organization and saves
some memory because only the top-level nsWindow now keeps IME states.

GetIMEComposition and RemoveIMEComposition are kept inside nsWindow
because they are not strictly related to IME events, and they are used
by some other event handlers in nsWindow.
Attachment #8671151 - Flags: review?(esawin)
GeckoEvent.KEY_EVENT and GeckoEvent.IME_EVENT used to count as UI events
for the hang monitor. We should count the new native calls as UI
events also, through this patch.
Attachment #8671153 - Flags: review?(esawin)
Remove obsolete GeckoEvent definitions from GeckoEvent.java and
AndroidJavaWrappers.cpp/h.
Attachment #8671154 - Flags: review?(snorp)
Attachment #8671149 - Flags: review?(esawin) → review+
Attachment #8671150 - Flags: review?(esawin) → review+
Comment on attachment 8671151 [details] [diff] [review]
Convert IME event handler in nsWindow to native calls (v1)

Review of attachment 8671151 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but it's becoming difficult to follow the subtle changes in semantics, do we have any tests for this?

::: widget/android/nsWindow.cpp
@@ +223,5 @@
> +        * Java controls the composition, and Gecko shadows the Java
> +           composition through update composition events
> +    */
> +
> +    struct IMEChange final {

Maybe IMETextChange would be better?
Attachment #8671151 - Flags: review?(esawin) → review+
Attachment #8671153 - Flags: review?(esawin) → review+
Comment on attachment 8671154 [details] [diff] [review]
Remove obsolete GeckoEvent definitions (v1)

Review of attachment 8671154 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM (assuming it builds).
Attachment #8671154 - Flags: review?(snorp) → review+
Assignee: nobody → nchen
Right now we call disposeNative on GeckoEditable in the
nsWindow::Natives destructor. However, we may still have pending native
calls in the event queue at that point, and these events will cause
exceptions when handled. This patch makes GeckoEditable call
disposeNative, after ensuring there's no pending calls.
Attachment #8674432 - Flags: review?(esawin)
Attachment #8674432 - Flags: review?(esawin) → review+
You need to log in before you can comment on or make changes to this bug.