Closed Bug 885692 Opened 11 years ago Closed 11 years ago

virtual keyboard layout switching API

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:koi+, firefox26 fixed)

RESOLVED FIXED
blocking-b2g koi+
Tracking Status
firefox26 --- fixed

People

(Reporter: jj.evelyn, Assigned: janjongboom)

References

Details

Attachments

(1 file, 4 obsolete files)

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).
nominate koi? since allowing 3rd-party keyboard installation is an important feature in v1.2.
blocking-b2g: --- → koi?
Blocks: 891673
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)
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)
(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: nobody → janjongboom
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)
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(); > };
Attachment #777053 - Attachment is obsolete: true
Attachment #777053 - Flags: review?(fabrice)
Attachment #777055 - Flags: review?(fabrice)
Has this API change been discussed and agreed on somewhere?
(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. :)
Attached patch 885692_v3.diff (obsolete) (deleted) — Splinter Review
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 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-
blocking-b2g: koi? → koi+
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)
Attachment #782538 - Flags: review?(fabrice) → review?(xyuan)
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-
Attached patch 885692_v5.diff (deleted) — Splinter Review
Thanks, learning every day :-) I have addressed your comments.
Attachment #782538 - Attachment is obsolete: true
Attachment #784370 - Flags: review?(xyuan)
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+
User Jan Jongboom <janjongboom@gmail.com> Bug 885692 - Implement InputMethodManager for virtual keyboard layout switching. r=yxl
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: