Closed
Bug 1323429
Opened 7 years ago
Closed 7 years ago
[geckoview] [e10s] Make IME work
Categories
(Core Graveyard :: Embedding: APIs, defect)
Core Graveyard
Embedding: APIs
Tracking
(firefox54 fixed)
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: esawin, Assigned: esawin)
References
Details
Attachments
(2 files, 3 obsolete files)
2.24 KB,
patch
|
jchen
:
review+
|
Details | Diff | Splinter Review |
5.56 KB,
patch
|
esawin
:
review+
|
Details | Diff | Splinter Review |
GV crashes on user input with e10s enabled.
Assignee | ||
Updated•7 years ago
|
Blocks: geckoview_mvp
Assignee | ||
Comment 1•7 years ago
|
||
This is a workaround to prevent it from crashing since IME{StateManager|ContentObserver} run in a different process.
Attachment #8830010 -
Flags: feedback?(nchen)
Comment 2•7 years ago
|
||
Comment on attachment 8830010 [details] [diff] [review] 0001-Bug-1323429-1.0-Remove-invalid-cross-process-calls.patch Review of attachment 8830010 [details] [diff] [review]: ----------------------------------------------------------------- We will regress IME if we remove these checks. Does IMEStateManager::GetFocusSelectionAndRoot fail? If so, maybe you can skip these checks only if IMEStateManager::GetFocusSelectionAndRoot fails (in which case we assume we are running under e10s).
Attachment #8830010 -
Flags: feedback?(nchen)
Assignee | ||
Comment 3•7 years ago
|
||
I assume this will regress some use cases, but could be OK as a temporary workaround. I'm assigning this bug to myself only to address some of the current IME blockers to allow for GV testing.
Assignee: nobody → esawin
Attachment #8830010 -
Attachment is obsolete: true
Attachment #8830080 -
Flags: review?(nchen)
Assignee | ||
Comment 4•7 years ago
|
||
Sorry, that's the correct patch version.
Attachment #8830080 -
Attachment is obsolete: true
Attachment #8830080 -
Flags: review?(nchen)
Attachment #8830082 -
Flags: review?(nchen)
Updated•7 years ago
|
Attachment #8830082 -
Flags: review?(nchen) → review+
Pushed by esawin@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/902da67acc3c [1.1] Don't verify IME root content with e10s enabled. r=jchen
Assignee | ||
Updated•7 years ago
|
Keywords: leave-open
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8830508 -
Flags: review?(nchen)
Comment 7•7 years ago
|
||
Comment on attachment 8830508 [details] [diff] [review] 0002-Bug-1323429-2.0-Remove-the-context-reference-from-Cl.patch Review of attachment 8830508 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/moz.build @@ +305,4 @@ > 'sqlite/SQLiteBridge.java', > 'sqlite/SQLiteBridgeException.java', > 'TouchEventInterceptor.java', > + 'util/Clipboard.java', We should move it out of util/ ::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/Clipboard.java @@ +7,4 @@ > import java.util.concurrent.SynchronousQueue; > > import org.mozilla.gecko.annotation.WrapForJNI; > +import org.mozilla.gecko.AppConstants.Versions; Not used? @@ +55,4 @@ > public void run() { > // In API Level 11 and above, CLIPBOARD_SERVICE returns android.content.ClipboardManager, > // which is a subclass of android.text.ClipboardManager. > + Context context = GeckoAppShell.getContext(); final, and use GeckoAppShell.getApplicationContext @@ +58,5 @@ > + Context context = GeckoAppShell.getContext(); > + if (context == null) { > + return; > + } > + final android.content.ClipboardManager cm = (android.content.ClipboardManager) context.getSystemService(Context.CLIPBOARD_SERVICE); Import android.content.ClipboardManager @@ +78,4 @@ > */ > @WrapForJNI(calledFrom = "gecko") > public static boolean hasText() { > + Context context = GeckoAppShell.getContext(); Ditto @@ +81,5 @@ > + Context context = GeckoAppShell.getContext(); > + if (context == null) { > + return false; > + } > + android.content.ClipboardManager cm = (android.content.ClipboardManager) context.getSystemService(Context.CLIPBOARD_SERVICE); final @@ +101,4 @@ > */ > @SuppressWarnings("deprecation") > static String getClipboardTextImpl() { > + Context context = GeckoAppShell.getContext(); Ditto @@ +104,5 @@ > + Context context = GeckoAppShell.getContext(); > + if (context == null) { > + return null; > + } > + android.content.ClipboardManager cm = (android.content.ClipboardManager) context.getSystemService(Context.CLIPBOARD_SERVICE); final
Attachment #8830508 -
Flags: review?(nchen) → review+
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/902da67acc3c
Assignee | ||
Comment 9•7 years ago
|
||
Addressed comments.
Attachment #8830508 -
Attachment is obsolete: true
Attachment #8830724 -
Flags: review+
Comment 10•7 years ago
|
||
Pushed by esawin@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/08a6ef48cebd [2.1] Remove the context reference from Clipboard. r=jchen
Assignee | ||
Updated•7 years ago
|
Keywords: leave-open
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/08a6ef48cebd
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Updated•5 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•