816912 - Keyboard Manager to support 3rd-party keyboard app.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P1)

Description

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

** For V2 - Keyboard **

In Gaia System, we would have a module called KeyboardManager, which will interpret the keyboard app's manifest and keep a list of installed input modules.

We would also need to update the settings app. to enable/disable a specific input module in the list.

Comment 1

[Evelyn Hung]

I have a WIP local branch for this issue, but I am trying a better way to launch a keyboard app. I tested to launch and destroy a keyboard app dynamically every time when a input filed is focused/blured, but it took 2-3 seconds to show a keyboard, too slow. I'll try other ways then.

Comment 2

[Evelyn Hung]

My on-going local branch: https://github.com/evelynhung/gaia/tree/keyboard-v2-test

Comment 3

[Evelyn Hung]

Attachment: point to https://github.com/mozilla-b2g/gaia/pull/9030

Comment 4

[Evelyn Hung]

(In reply to Evelyn Hung [:evelyn] from comment #3)
> Created attachment 734390 [details]
> point to https://github.com/mozilla-b2g/gaia/pull/9030

To test this patch, please also apply patch in the bug 858383.

Comment 5

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

Comment on attachment 734390 [details]
point to https://github.com/mozilla-b2g/gaia/pull/9030

Remove r? based on offline discussion.

Comment 6

[Evelyn Hung]

Comment on attachment 734390 [details]
point to https://github.com/mozilla-b2g/gaia/pull/9030

updated and ask review again.

Comment 7

[David Flanagan [:djf]]

Evelyn,

Since this is a v2 feature, I don't think I'm allowed to spend time reviewing it right now: https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.gaia/E1xniLLxPoc

Sorry!

Comment 8

[David Flanagan [:djf]]

Evelyn,

I'm sorry I've let this review sit for so long.  I'm finally getting clear of v1.1 bugs and will be able to start thinking about v1.2.

Is this patch still the one that needs review, or has the code or the design changed since you wrote this?

Comment 9

[Evelyn Hung]

Comment on attachment 734390 [details]
point to https://github.com/mozilla-b2g/gaia/pull/9030

Clear review request here because it's out of date. Gary will rebase the code and submit a new review request again. Thanks everyone!

Comment 10

[Jan Jongboom [:janjongboom] (inactive)]

Great. FYI we landed a patch that allows you to determine keyboard apps by checking `manifest.role === 'keyboard'`.

Comment 11

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Attachment: pull request: https://github.com/mozilla-b2g/gaia/pull/11528

This is the pull request to add 3rdParty keyboard support.
Please be informed this change would need Gecko m-c to test with.

For this bug, please help review 2742988283856b97c4becb5cac667795bc3b2c7e only, the other 2 commits will be reviewed by other module peers/owners.

Comment 12

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

Comment on attachment 790121 [details]
pull request: https://github.com/mozilla-b2g/gaia/pull/11528

Good job! Let's discuss this face to face.

The code is mostly correct but before we land this patch I would like to see questions being answered either by more meaningful comments or better naming of the functions.

The code is also too complex and it would be better to be further split into helper classes, accompany with test cases. I don't think it's possible to throughly test this class as-is so it will be a problem further down the road. Let's make sure we do that before more feature is being added onto the code.

Comment 13

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

Landed to Gaia master
bc8c1c962addd419b48ba12c6d8ddabdf9805391

Comment 14

[James Lal [:lightsofapollo]]

backed out as part of the general backout of the 3rd party keyboard customizations (https://github.com/mozilla-b2g/gaia/commit/42aa1e2ccd22e464bffe6b22ee1182cece5dc2dc) Due to b2g-desktop failures (fail to start) https://travis-ci.org/mozilla-b2g/gaia/builds/10571231#L129.

Comment 15

[Ed Morley [:emorley]]

Something in this gaia.json auto-commit:
https://hg.mozilla.org/integration/b2g-inbound/rev/31ca106e662c

...broke the gaia unit tests, eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=27215367&tree=B2g-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=27214598&tree=B2g-Inbound

The auto-commit corresponds to:
https://hg.mozilla.org/integration/gaia-central/pushloghtml?fromchange=c258483698afbf2c2b7f27f8fa16922d0ee48880&tochange=75e0fbd934e2c835db9ecd10ec6b2900b357d53c

So this and the others in that push have been backed out:
https://github.com/mozilla-b2g/gaia/commit/c2f23f132accb38af4a46f9171bc6be25fdb99a1
https://github.com/mozilla-b2g/gaia/commit/9a8098e8a6b86a32da0eef08e507ab689435dfee
https://github.com/mozilla-b2g/gaia/commit/884d0af45bc605c8e943e3080b2a909461795953
https://github.com/mozilla-b2g/gaia/commit/6a653be7c8b34f263342ead2037b4f8f3c2aff0e
https://github.com/mozilla-b2g/gaia/commit/cfcb28b5069054c1ad631c5bf4ed9c2a94e06d8b

Please run the gaia unit tests before pushing if possible! :-)

Comment 16

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

Merged to Gaia master with this rev,
https://github.com/mozilla-b2g/gaia/commit/9fb5802df60a9081846d704def01df814ed8fbd4