Closed
Bug 1416918
Opened 8 years ago
Closed 8 years ago
Provide InputConnection/key-event API for GeckoSession
Categories
(GeckoView :: IME, enhancement)
Tracking
(firefox59 fixed)
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: jchen, Assigned: jchen)
References
Details
Attachments
(6 files)
59 bytes,
text/x-review-board-request
|
esawin
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
esawin
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
esawin
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
esawin
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
jchen
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
esawin
:
review+
|
Details |
GeckoSession should provide an API for getting an InputConnection instance and for handling key events. That will involve changing GeckoEditable because it relies on GeckoView right now. Furthermore, it would be even better if we can get rid of the WrapForJNI annotations from GeckoEditable itself, to prepare for the eventual process split. Finally, GeckoView will use the new API to handle methods like onCreateInputConnection.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → nchen
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8935428 [details]
Bug 1416918 - 1. Add TextInputController;
https://reviewboard.mozilla.org/r/206324/#review212086
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoSession.java:559
(Diff revision 1)
> + ThreadUtils.assertOnUiThread();
> +
> + if (mTextInput == null) {
> + mTextInput = new TextInputController(this);
> + if (mWindow != null) {
> + mTextInput.onWindowReady(mNativeQueue, mWindow);
Can we guarantee that mWindow is set?
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/TextInputController.java:18
(Diff revision 1)
> +import android.view.KeyEvent;
> +import android.view.View;
> +import android.view.inputmethod.EditorInfo;
> +import android.view.inputmethod.InputConnection;
> +
> +public final class TextInputController {
Please add a class description.
::: widget/android/nsWindow.cpp
(Diff revision 1)
>
> void
> nsWindow::GeckoViewSupport::Attach(const GeckoSession::Window::LocalRef& inst,
> jni::Object::Param aView)
> {
> - // Associate our previous GeckoEditable with the new GeckoView.
Can we remove this now?
Attachment #8935428 -
Flags: review?(esawin) → review+
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8935429 [details]
Bug 1416918 - 2. Make GeckoEditable/GeckoInputConnection work with TextInputController;
https://reviewboard.mozilla.org/r/206326/#review212100
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditable.java:46
(Diff revision 1)
> GeckoEditable implements only some functions of Editable
> The field mText contains the actual underlying
> SpannableStringBuilder/Editable that contains our text.
> */
> -final class GeckoEditable extends IGeckoEditableParent.Stub
> - implements InvocationHandler, Editable, GeckoEditableClient {
> +/* package */ final class GeckoEditable extends IGeckoEditableParent.Stub
> + implements InvocationHandler,
To improve readability, I would prefer moving both, extends and implements, into a new line but with the base 4 spaces indentation. This will allow more consistent indentation across classes, including those with absurdly long names.
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoInputConnection.java:48
(Diff revision 1)
> import android.view.inputmethod.ExtractedTextRequest;
> import android.view.inputmethod.InputConnection;
> import android.view.inputmethod.InputMethodManager;
>
> -class GeckoInputConnection
> - extends BaseInputConnection
> +/* package */ class GeckoInputConnection extends BaseInputConnection
> + implements TextInputController.Delegate,
See above comment.
Attachment #8935429 -
Flags: review?(esawin) → review+
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8935430 [details]
Bug 1416918 - 3. Use TextInputController in GeckoView;
https://reviewboard.mozilla.org/r/206328/#review212104
Attachment #8935430 -
Flags: review?(esawin) → review+
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8935431 [details]
Bug 1416918 - 4. Remove unused code;
https://reviewboard.mozilla.org/r/206330/#review212106
Attachment #8935431 -
Flags: review?(esawin) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•8 years ago
|
||
mozreview-review |
Comment on attachment 8935869 [details]
Bug 1416918 - 4b. Move GeckoEditable{Client,Listener} to TextInputController;
https://reviewboard.mozilla.org/r/206726/#review212990
Attachment #8935869 -
Flags: review?(esawin) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 23•8 years ago
|
||
mozreview-review |
Comment on attachment 8935432 [details]
Bug 1416918 - 5. Update generated bindings;
https://reviewboard.mozilla.org/r/206332/#review213372
Attachment #8935432 -
Flags: review+
Comment 24•8 years ago
|
||
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e18c58d0320
1. Add TextInputController; r=esawin
https://hg.mozilla.org/integration/autoland/rev/29a3c1e94980
2. Make GeckoEditable/GeckoInputConnection work with TextInputController; r=esawin
https://hg.mozilla.org/integration/autoland/rev/39f1117294d1
3. Use TextInputController in GeckoView; r=esawin
https://hg.mozilla.org/integration/autoland/rev/40e3669c4a30
4. Remove unused code; r=esawin
https://hg.mozilla.org/integration/autoland/rev/a0b80fff3ab5
4b. Move GeckoEditable{Client,Listener} to TextInputController; r=esawin
https://hg.mozilla.org/integration/autoland/rev/cc7d974a7a95
5. Update generated bindings; r=jchen
Comment 25•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3e18c58d0320
https://hg.mozilla.org/mozilla-central/rev/29a3c1e94980
https://hg.mozilla.org/mozilla-central/rev/39f1117294d1
https://hg.mozilla.org/mozilla-central/rev/40e3669c4a30
https://hg.mozilla.org/mozilla-central/rev/a0b80fff3ab5
https://hg.mozilla.org/mozilla-central/rev/cc7d974a7a95
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Updated•7 years ago
|
Product: Firefox for Android → GeckoView
Updated•7 years ago
|
Target Milestone: Firefox 59 → mozilla59
Comment 26•3 years ago
|
||
Moving some cursor and key event bugs to the new GeckoView::IME component.
Component: General → IME
You need to log in
before you can comment on or make changes to this bug.
Description
•