Closed
Bug 885692
Opened 11 years ago
Closed 11 years ago
virtual keyboard layout switching API
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(blocking-b2g:koi+, firefox26 fixed)
People
(Reporter: jj.evelyn, Assigned: janjongboom)
References
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
xyuan
:
review+
|
Details | Diff | Splinter Review |
We need a API for 3rd-party keyboard app to inform System app switching to another keyboard layout or another keyboard app.
The API can be used as a system fallback control: if a 3rd-party keyboard app didn't implement a button for user switching to another keyboard app, System app can use this API and provide a UI (on utility tray per sepc definition) to user to force it happen. (like what Android's way).
Reporter | ||
Comment 1•11 years ago
|
||
nominate koi? since allowing 3rd-party keyboard installation is an important feature in v1.2.
blocking-b2g: --- → koi?
Reporter | ||
Updated•11 years ago
|
Blocks: keyboard-api, 3rd-party-keyboard
Assignee | ||
Comment 2•11 years ago
|
||
Evelyn, I don't really see why we'd need this? What would a 3rd party keyboard need to tell something to the system?
Flags: needinfo?(ehung)
Reporter | ||
Comment 3•11 years ago
|
||
If it wants to implement a 'globe' button within its layout, then it needs this API to tell System app taking user to another keyboard layout/app.
Flags: needinfo?(ehung)
Reporter | ||
Comment 4•11 years ago
|
||
(In reply to Evelyn Hung [:evelyn] from comment #3)
> If it wants to implement a 'globe' button within its layout, then it needs
> this API to tell System app taking user to another keyboard layout/app.
Checking UX draft on bug 891678 will help you know the case. (in page 9)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → janjongboom
Assignee | ||
Comment 5•11 years ago
|
||
This patch adds `switchLayout()` function to navigator.mozKeyboard. When calling it emits a chromeEvent that can be handled by the keyboard_manager in gaia.
```
window.addEventListener('mozChromeEvent', function(ev) {
if (ev.detail.type === 'keyboard-switch-layout') {
// show keyboard switching UI
console.log('Show keyboard switching UI');
}
});
```
Attachment #777053 -
Flags: review?(fabrice)
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 777053 [details] [diff] [review]
Add switchLayout() method to keyboard that emits chromeEvent for the system app
># HG changeset patch
># Parent 582ffcd0459aebedc63940af9ec25675836aee17
># User Jan Jongboom <janjongboom@gmail.com>
>Bug 885692 - Add switchLayout() method to keyboard that emits chromeEvent for the system app
>
>
>diff --git a/b2g/components/Keyboard.jsm b/b2g/components/Keyboard.jsm
>--- a/b2g/components/Keyboard.jsm
>+++ b/b2g/components/Keyboard.jsm
>@@ -16,17 +16,17 @@ Cu.import("resource://gre/modules/XPCOMU
>
> XPCOMUtils.defineLazyServiceGetter(this, "ppmm",
> "@mozilla.org/parentprocessmessagemanager;1", "nsIMessageBroadcaster");
>
> let Keyboard = {
> _messageManager: null,
> _messageNames: [
> 'SetValue', 'RemoveFocus', 'SetSelectedOption', 'SetSelectedOptions',
>- 'SetSelectionRange', 'ReplaceSurroundingText'
>+ 'SetSelectionRange', 'ReplaceSurroundingText', 'SwitchLayout'
> ],
>
> get messageManager() {
> if (this._messageManager && !Cu.isDeadWrapper(this._messageManager))
> return this._messageManager;
>
> throw Error('no message manager set');
> },
>@@ -115,16 +115,19 @@ let Keyboard = {
> this.setSelectedOption(msg);
> break;
> case 'Keyboard:SetSelectionRange':
> this.setSelectionRange(msg);
> break;
> case 'Keyboard:ReplaceSurroundingText':
> this.replaceSurroundingText(msg);
> break;
>+ case 'Keyboard:SwitchLayout':
>+ this.switchLayout();
>+ break;
> }
> },
>
> handleFormsInput: function keyboardHandleFormsInput(msg) {
> this.messageManager = msg.target.QueryInterface(Ci.nsIFrameLoaderOwner)
> .frameLoader.messageManager;
>
> ppmm.broadcastAsyncMessage('Keyboard:FocusChange', msg.data);
>@@ -155,12 +158,20 @@ let Keyboard = {
>
> removeFocus: function keyboardRemoveFocus() {
> this.messageManager.sendAsyncMessage('Forms:Select:Blur', {});
> },
>
> replaceSurroundingText: function keyboardReplaceSurroundingText(msg) {
> this.messageManager.sendAsyncMessage('Forms:ReplaceSurroundingText',
> msg.data);
>+ },
>+
>+ switchLayout: function keyboardSwitchLayout() {
>+ let browser = Services.wm.getMostRecentWindow("navigator:browser");
>+ browser.shell.sendChromeEvent({
>+ type: "keyboard-switch-layout"
>+ });
> }
> };
>
> Keyboard.init();
>diff --git a/b2g/components/MozKeyboard.js b/b2g/components/MozKeyboard.js
>--- a/b2g/components/MozKeyboard.js
>+++ b/b2g/components/MozKeyboard.js
>@@ -189,12 +189,16 @@ MozKeyboard.prototype = {
> handler.handleEvent(evt);
> }
> },
>
> observe: function mozKeyboardObserve(subject, topic, data) {
> let wId = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
> if (wId == this.innerWindowID)
> this.uninit();
>+ },
>+
>+ switchLayout: function mozKeyboardSwitchLayout() {
>+ cpmm.sendAsyncMessage('Keyboard:SwitchLayout', {});
> }
> };
>
> this.NSGetFactory = XPCOMUtils.generateNSGetFactory([MozKeyboard]);
>diff --git a/b2g/components/b2g.idl b/b2g/components/b2g.idl
>--- a/b2g/components/b2g.idl
>+++ b/b2g/components/b2g.idl
>@@ -70,9 +70,18 @@ interface nsIB2GKeyboard : nsISupports
> * @param beforeLength The number of characters to be deleted before the
> * beginning of the current selection range. Defaults to 0.
> * @param afterLength The number of characters to be deleted after the
> * beginning of the current selection range. Defaults to 0.
> */
> void replaceSurroundingText(in DOMString text,
> [optional] in long beforeLength,
> [optional] in long afterLength);
>+
>+ /*
>+ * Indicate that the user wants to switch keyboard layout.
>+ *
>+ * This method can be invoked from client side keyboard code and will emit
>+ * a chromeEvent that can be handled by the system to present the user
>+ * with a choice of keyboard layouts available.
>+ */
>+ void switchLayout();
> };
Assignee | ||
Comment 7•11 years ago
|
||
Attachment #777053 -
Attachment is obsolete: true
Attachment #777053 -
Flags: review?(fabrice)
Attachment #777055 -
Flags: review?(fabrice)
Comment 8•11 years ago
|
||
Has this API change been discussed and agreed on somewhere?
Assignee | ||
Comment 9•11 years ago
|
||
No, at least it's not on https://wiki.mozilla.org/WebAPI/KeboardIME.
Assignee | ||
Comment 10•11 years ago
|
||
Discussion going on here https://groups.google.com/forum/#!topic/mozilla.dev.webapi/nZLOEvEL3Tk
Reporter | ||
Comment 11•11 years ago
|
||
(In reply to Jan Jongboom [:janjongboom] from comment #9)
> No, at least it's not on https://wiki.mozilla.org/WebAPI/KeboardIME.
The wiki page is up-to-date now, but it's still not a confirmed version.
Discussion on the mailing list is on-going, feedback welcome. :)
Assignee | ||
Comment 12•11 years ago
|
||
This patch contains the changes discussed in the mailing list, I have the new methods in their own interface at the moment, if we want them in mozKeyboard it's an easy change, but I'm on PTO from tomorrow until 7/29, so that's up to you :-)
Usage in keyboard_manager.js:
window.addEventListener('mozChromeEvent', function(ev) {
if (ev.detail.type === 'input-method-show-picker') {
console.log('Show keyboard switching UI');
}
else if (ev.detail.type === 'input-method-switch-to-next') {
console.log('Switch to next input method');
}
});
Attachment #777055 -
Attachment is obsolete: true
Attachment #777055 -
Flags: review?(fabrice)
Attachment #777691 -
Flags: review?(xyuan)
Comment 13•11 years ago
|
||
Comment on attachment 777691 [details] [diff] [review]
885692_v3.diff
Review of attachment 777691 [details] [diff] [review]:
-----------------------------------------------------------------
I'm very excited that you're working on this, but some polish is needed.
BTW, Fabrice should be more suitable to review the patch.
::: b2g/components/Keyboard.jsm
@@ +169,5 @@
> msg.data);
> + },
> +
> + showInputMethodPicker: function keyboardShowInputMethodPicker() {
> + dump('showInputMethodPicker\n');
nit: remove unnecessary log code
@@ +177,5 @@
> + });
> + },
> +
> + switchToNextInputMethod: function keyboardSwitchToNextInputMethod() {
> + dump('switchToNextInputMethod\n')
nit: remove unnecessary log code
::: b2g/components/MozKeyboard.js
@@ +202,5 @@
> +MozInputMethodManager.prototype = {
> + classID: Components.ID("{7e9d7280-ef86-11e2-b778-0800200c9a66}"),
> +
> + QueryInterface: XPCOMUtils.generateQI([
> + Ci.nsIInputMethodManager
You'd better add Ci.nsIDOMGlobalPropertyInitializer. Please refer to https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIDOMGlobalPropertyInitializer for more.
@@ +223,5 @@
> + principal.origin + "\n");
> + return null;
> + }
> +
> + Services.obs.addObserver(this, "inner-window-destroyed", false);
This observer is useless. It should be removed with this._window, this._utils and this.innerWindowsID, as you never use them.
@@ +231,5 @@
> + .getInterface(Ci.nsIDOMWindowUtils);
> + this.innerWindowID = this._utils.currentInnerWindowID;
> + },
> +
> + uninit: function mozInputMethodManagerUninit() {
This method has never been called and may be removed.
::: b2g/components/b2g.idl
@@ +78,5 @@
> };
> +
> +// Manages the list of IMEs, enables/disables IME and switches to an IME.
> +[scriptable, uuid(e51a6fa0-ef85-11e2-b778-0800200c9a66)]
> +interface nsIInputMethodManager : nsISupports
nit: only one space is needed before ':'
Attachment #777691 -
Flags: review?(xyuan) → review-
Updated•11 years ago
|
blocking-b2g: koi? → koi+
Assignee | ||
Comment 14•11 years ago
|
||
This patch contains the API as currently proposed for InputMethodManager (part of navigator.mozInputMethod).
Attachment #777691 -
Attachment is obsolete: true
Attachment #782538 -
Flags: review?(fabrice)
Assignee | ||
Updated•11 years ago
|
Attachment #782538 -
Flags: review?(fabrice) → review?(xyuan)
Comment 15•11 years ago
|
||
Comment on attachment 782538 [details] [diff] [review]
Implement InputMethod & InputMethodManager interface according to API spec
Review of attachment 782538 [details] [diff] [review]:
-----------------------------------------------------------------
You're almost there and just need a few fixes.
::: b2g/components/MozKeyboard.js
@@ +208,5 @@
> + classID: Components.ID("{7e9d7280-ef86-11e2-b778-0800200c9a66}"),
> +
> + QueryInterface: XPCOMUtils.generateQI([
> + Ci.nsIInputMethodManager,
> + Ci.nsIDOMGlobalPropertyInitializer
If we don't expose Ci.nsIDOMGlobalPropertyInitializer as navigator property, we don't need to implement Ci.nsIDOMGlobalPropertyInitializer. So this interface as well as its implementation `init: function mozInputMethodManagerInit(win)` should be removed.
@@ +219,5 @@
> + "flags": Ci.nsIClassInfo.DOM_OBJECT,
> + "classDescription": "B2G Input Method Manager"
> + }),
> +
> + init: function mozInputMethodManagerInit(win) {
This method could be removed.
@@ +241,5 @@
> +
> + next: function() {
> + cpmm.sendAsyncMessage('Keyboard:SwitchToNextInputMethod', {});
> + },
> +
nit: remove trailing white spaces.
@@ +245,5 @@
> +
> + supportsSwitching: function() {
> + return true;
> + },
> +
nit: remove trailing white spaces.
@@ +286,5 @@
> + }
> +
> + this._window = win;
> + this._utils = win.QueryInterface(Ci.nsIInterfaceRequestor)
> + .getInterface(Ci.nsIDOMWindowUtils);
this._window and this._utils should be removed if not used.
Attachment #782538 -
Flags: review?(xyuan) → review-
Assignee | ||
Comment 16•11 years ago
|
||
Thanks, learning every day :-) I have addressed your comments.
Attachment #782538 -
Attachment is obsolete: true
Attachment #784370 -
Flags: review?(xyuan)
Comment 17•11 years ago
|
||
Comment on attachment 784370 [details] [diff] [review]
885692_v5.diff
Review of attachment 784370 [details] [diff] [review]:
-----------------------------------------------------------------
r=me. Well done!
Attachment #784370 -
Flags: review?(xyuan) → review+
Assignee | ||
Comment 18•11 years ago
|
||
User Jan Jongboom <janjongboom@gmail.com>
Bug 885692 - Implement InputMethodManager for virtual keyboard layout switching. r=yxl
Keywords: checkin-needed
Comment 19•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/a850d7025a3c
All hail our new b2g-inbound overlords!
Keywords: checkin-needed
Assignee | ||
Comment 20•11 years ago
|
||
\o/
Comment 21•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
status-firefox26:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•