842436 - Keyboard API: There should only be one keyboard active, and Gecko should block interaction from non-active keyboards

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect, P1)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Keyboard API should prevent input from a non-visible keyboard app.

We need this in order to support multiple keyboards app. See bug 838308 comment 32.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

No, I don't think we should use the visibility state. If someone somehow opens a installed 3rd party keyboard app it shouldn't be able inject characters into other apps.

We should instead keep track of which app we've currently opened in the keyboard area. Only that app should be able to interact with the keyboard API.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Jonas Sicking (:sicking) from comment #1)
> No, I don't think we should use the visibility state. If someone somehow
> opens a installed 3rd party keyboard app it shouldn't be able inject
> characters into other apps.
> 
> We should instead keep track of which app we've currently opened in the
> keyboard area. Only that app should be able to interact with the keyboard
> API.

But only keyboard apps and system app will have the permission, and window manager will block launching a keyboard app in the main area.

Are you suggesting getting yet another mozContentEvent/mozChromeEvent pair for Gaia system to communicate with Gecko? How is that any safer than visibility state?

Comment 3

[[:fabrice] Fabrice Desré]

Tim, Gaia system app will need some way to know which app is the "real" keyboard app. If it uses a setting for that, we can track the keyboard app on the gecko side and only allow events from this app. Having an explicit tracking is safer than the visibility state which is quite volatile.

Comment 4

[Evelyn Hung]

Keyboard manager in System app only use a (Javascript) object to track currently working keyboard app. Actually, we manage all installed and running keyboard apps at Gaia, which I think it's a little bit weird because gecko knows nothing about the state of these keyboard apps. 

Is it possible to extend keyboard API so we(Gaia) could:
1. get all installed keyboard apps
2. be notified when any keyboard app is installed/uninitalled/crashed
3. get running keyboard apps
4. get currently working(showing) keyboard app

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Evelyn Hung [:evelyn] from comment #4)
> Keyboard manager in System app only use a (Javascript) object to track
> currently working keyboard app. Actually, we manage all installed and
> running keyboard apps at Gaia, which I think it's a little bit weird because
> gecko knows nothing about the state of these keyboard apps. 

The above and below are contradicting. Do you need Gecko to supply information to Gaia, or vise versa?

> Is it possible to extend keyboard API so we(Gaia) could:
> 1. get all installed keyboard apps
Are you thinking about creating special keyboard layout registry? Any reason we couldn't use mozApp?

> 2. be notified when any keyboard app is installed/uninitalled/crashed
mozBrowser API and its events?

> 3. get running keyboard apps
> 4. get currently working(showing) keyboard app

MozKeyboard API could certainly be enthroned to do what Keyboard Manager is suppose to do, but I thought we don't want that.

Comment 6

[Yuan Xulei [:yxl] (Leave from May 18)]

Yes, we could extend the keyboard API.

Actually in the proposed API(https://wiki.mozilla.org/WebAPI/KeboardIME), we declared the interface `InputMethodManager` to handle the keyboard management stuff. We can implement this interface to extend the keyboard API.

But I don't think we need provide all the functions below as APIs. As Tim mentioned, it will be better to handle some of functions through events if only system app needs these functions.

(In reply to Evelyn Hung [:evelyn] from comment #4)
> Keyboard manager in System app only use a (Javascript) object to track
> currently working keyboard app. Actually, we manage all installed and
> running keyboard apps at Gaia, which I think it's a little bit weird because
> gecko knows nothing about the state of these keyboard apps. 
> 
> Is it possible to extend keyboard API so we(Gaia) could:
> 1. get all installed keyboard apps
Implement an API similar to that of getting all installed apps.

> 2. be notified when any keyboard app is installed/uninitalled/crashed
by events

> 3. get running keyboard apps
yes, we could, but does gecko need such information?

> 4. get currently working(showing) keyboard app
Yes, we could.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

Forgive my jump in because I may not know the full story.
But IMO we shouldn't rely on visible state too heavily to do some other irrelevant work.

For example, audio service now heavily binds to visible state and we are trying to remove most audio competing work in gecko because it's inactively notified by gaia system app with BrowserIframe.setVisible() to do the decision. This is totally wrong.

(In my dream) One day we may support multi-window, and thus the primary window and the secondary window are all "visible". How could you tell from them? One must be "active" and another isn't.

I agree what Jonas says here, if we are improving/enhancing keyboard implementation, why not create a new focus-like function/event? I mean, visible state just isn't focusing state. But yes, a background window shouldn't do anything harmful to foreground window (without permission).

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Alive Kuo [:alive] (PTO during 3/27~4/7) from comment #7)

You are 100% correct. I've not think of the impact on have everything bind to setVisible().

Comment 9

[Evelyn Hung]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (PTO Mar 22 - Apr 7) from comment #5)
> (In reply to Evelyn Hung [:evelyn] from comment #4)
> > Keyboard manager in System app only use a (Javascript) object to track
> > currently working keyboard app. Actually, we manage all installed and
> > running keyboard apps at Gaia, which I think it's a little bit weird because
> > gecko knows nothing about the state of these keyboard apps. 
> 
> The above and below are contradicting. Do you need Gecko to supply
> information to Gaia, or vise versa?
Gecko supply information to Gaia. 
> 
> > Is it possible to extend keyboard API so we(Gaia) could:
> > 1. get all installed keyboard apps
> Are you thinking about creating special keyboard layout registry? Any reason
> we couldn't use mozApp?
we could, but I was wondering if we could have a straightforward way instead of enumerating all apps. 
> 
> > 2. be notified when any keyboard app is installed/uninitalled/crashed
> mozBrowser API and its events?
same above, we have app installed/uninstalled event hook and mozbrowser crash event, but again, we should monitor *all* apps and filter out keyboard app.
> 
> > 3. get running keyboard apps
> > 4. get currently working(showing) keyboard app
> 
> MozKeyboard API could certainly be enthroned to do what Keyboard Manager is
> suppose to do, but I thought we don't want that.
I was thinking how much management effort should be left in Gaia, could we have more specific API to avoid redundant work in both System app and Settings app?
But after offline discussion with Tim, I think I'm convinced that we should define API in a general way, to simplify Gecko work. 
So let's move forward with powerful(?) keyboard manager in Setting app.

Comment 10

[Evelyn Hung]

(In reply to Evelyn Hung [:evelyn] from comment #9)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (PTO Mar 22 - Apr 7)
> from comment #5)
> > (In reply to Evelyn Hung [:evelyn] from comment #4)
> > > Keyboard manager in System app only use a (Javascript) object to track
> > > currently working keyboard app. Actually, we manage all installed and
> > > running keyboard apps at Gaia, which I think it's a little bit weird because
> > > gecko knows nothing about the state of these keyboard apps. 
> > 
> > The above and below are contradicting. Do you need Gecko to supply
> > information to Gaia, or vise versa?
> Gecko supply information to Gaia. 
> > 
> > > Is it possible to extend keyboard API so we(Gaia) could:
> > > 1. get all installed keyboard apps
> > Are you thinking about creating special keyboard layout registry? Any reason
> > we couldn't use mozApp?
> we could, but I was wondering if we could have a straightforward way instead
> of enumerating all apps. 
> > 
> > > 2. be notified when any keyboard app is installed/uninitalled/crashed
> > mozBrowser API and its events?
> same above, we have app installed/uninstalled event hook and mozbrowser
> crash event, but again, we should monitor *all* apps and filter out keyboard
> app.
> > 
> > > 3. get running keyboard apps
> > > 4. get currently working(showing) keyboard app
> > 
> > MozKeyboard API could certainly be enthroned to do what Keyboard Manager is
> > suppose to do, but I thought we don't want that.
> I was thinking how much management effort should be left in Gaia, could we
> have more specific API to avoid redundant work in both System app and
> Settings app?
> But after offline discussion with Tim, I think I'm convinced that we should
> define API in a general way, to simplify Gecko work. 
> So let's move forward with powerful(?) keyboard manager in Setting app.
Sorry, correct, it's in System app.

Comment 11

[Evelyn Hung]

(In reply to Yuan Xulei [:yxl] from comment #6)
> Yes, we could extend the keyboard API.
> 
> Actually in the proposed API(https://wiki.mozilla.org/WebAPI/KeboardIME), we
> declared the interface `InputMethodManager` to handle the keyboard
> management stuff. We can implement this interface to extend the keyboard API.
> 
> But I don't think we need provide all the functions below as APIs. As Tim
> mentioned, it will be better to handle some of functions through events if
> only system app needs these functions.
> 
> (In reply to Evelyn Hung [:evelyn] from comment #4)
> > Keyboard manager in System app only use a (Javascript) object to track
> > currently working keyboard app. Actually, we manage all installed and
> > running keyboard apps at Gaia, which I think it's a little bit weird because
> > gecko knows nothing about the state of these keyboard apps. 
> > 
> > Is it possible to extend keyboard API so we(Gaia) could:
> > 1. get all installed keyboard apps
> Implement an API similar to that of getting all installed apps.
> 
> > 2. be notified when any keyboard app is installed/uninitalled/crashed
> by events
> 
> > 3. get running keyboard apps
> yes, we could, but does gecko need such information?
I think gecko should know. because of the issue we are talking about:
"There should only be one keyboard active, and Gecko should block interaction from non-active keyboards"
"running keyboard" = active keyboard.
> 
> > 4. get currently working(showing) keyboard app
> Yes, we could.

Comment 12

[Yuan Xulei [:yxl] (Leave from May 18)]

Hi Evelyn,
What's the difference between 3 and 4?

Comment 13

[Evelyn Hung]

> > > 3. get running keyboard apps
> > yes, we could, but does gecko need such information?
> I think gecko should know. because of the issue we are talking about:
> "There should only be one keyboard active, and Gecko should block
> interaction from non-active keyboards"
> "running keyboard" = active keyboard.
I must be tired because it's 1am here...
correct myself again, I actually was answering #4 below, 
and "working(showing) keyboard app" = "active keyboard".
Not sure whether Gecko should know all running (may in the background, i.e. inactive)
in this case.
> > 
> > > 4. get currently working(showing) keyboard app
> > Yes, we could.

Comment 14

[Evelyn Hung]

(In reply to Yuan Xulei [:yxl] from comment #12)
> Hi Evelyn,
> What's the difference between 3 and 4?
see my comment 13, sorry about my mistake. :(

Comment 15

[Yuan Xulei [:yxl] (Leave from May 18)]

After discussed with Tim, Rudy and Evely, we worked out a preliminary solution:

System App is responsible for telling Gecko which is the active keyboard and activating the keyboard. 

Since each keyboard app is loaded into a mozbrowser frame(https://developer.mozilla.org/en-US/docs/WebAPI/Browser), a specified method of mozbrowser
  `setInputMethodActive(boolean)`
is added to enable system app to activate the keyboard app frame.

When the active keyboard is changed, system app should first call `setInputMethodActive(true)` to enable the active one. Then calls `setInputMethodActive(false)' to block other keyboards.

MozKeyboard should block interaction from keyboard app if setInputMethodActive(true) is not called.

Comment 16

[Ivan Tsay (:ITsay)]

Since this block the keyboard privilege, flag it to koi+. Also add keyword in whiteboard for EPM tracking and wiki site: https://wiki.mozilla.org/FirefoxOS/SprintStatus#Systems-Platform

Comment 17

[Yuan Xulei [:yxl] (Leave from May 18)]

cc Stefan to help with security review.

Comment 18

[Ivan Tsay (:ITsay)]

Hi Xulei,

So this one is mainly depends on 905573. As long as 905573 is fixed, this one is also fixed? Or does this need extra effort to be completed. Thank you!

Comment 19

[Ivan Tsay (:ITsay)]

Update keyword in white board for the status query more precise

Comment 20

[Yuan Xulei [:yxl] (Leave from May 18)]

905573(In reply to Ivan Tsay (:ITsay) from comment #18)
> Hi Xulei,
> 
> So this one is mainly depends on 905573. As long as 905573 is fixed, this
> one is also fixed? Or does this need extra effort to be completed. Thank you!
After 905573 is fixed, most part of this bug is fixed, but there may need a slight change on gaia.

Comment 21

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/12223

Pointer to Github pull-request

Comment 22

[Yuan Xulei [:yxl] (Leave from May 18)]

Comment on attachment 804552 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/12223

The patch uses iframe.setInputMethodActive to set the active input method iframe and disable others to enforce security.

Rudy, feel free to take the bug if you have a better way to implement it. I want the gaia part of this bug lands before the gecko part of Bug 905573(the implementation of iframe.setInputMethodActive). Because the patch of Bug 905573 will break gaia keyboard function without fixing this one.

Comment 23

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 804552 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/12223

r+, let's land this first if it won't break Gaia.

Yuan,

Thanks for your work.

Comment 24

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Merged to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/56439c30f4101dd5e4bcd09575ada05a6ef5ac15