1054942 - Dissect |KeyboardManager.updateLayouts()|

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Input Mgmt, defect)

Description

[John Lu [:mnjul][Please NI or I'll miss]]

|KeyboardManager.updateLayouts()| is currently one big function with several inner functions. Let's dissect it.

Comment 1

[John Lu [:mnjul][Please NI or I'll miss]]

Working on this.

Comment 2

[John Lu [:mnjul][Please NI or I'll miss]]

WIP on this: https://github.com/mnjul/gaia/tree/bug_1054942_dissect_keyboardmanager_updatelayout . Haven't modified unit tests accordingly.

Comment 3

[John Lu [:mnjul][Please NI or I'll miss]]

My unit tests-writing is blocked by bug 1055908 since the unit tests to add in bug 1055908 are right dealing with codes in |updateLayout()| .

Comment 4

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: Patch (PR @ GitHub)

Let's have it tested!

Comment 5

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8476467 [details] [review]
Patch (PR @ GitHub)

Hi Tim,

Here's another refactoring work to review, thanks. Layout-related chores in |KeyboardManager.updateLayouts()| were extracted to a new sub-module, and were separated into smaller functions with descriptive function names. Layout bookkeeping was moved to the sub-module too.

For unit tests, the "module functional unit-test" sense for KeyboardManager is retained (i.e. keyboard_manager_test depends on a fully-functional InputLayouts and no tests were removed from keyboard_manager_test). For InputLayouts per-function tests are performed.

Comment 6

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

Comment on attachment 8476467 [details] [review]
Patch (PR @ GitHub)

I didn't try to look into the patch line-by-line but I think this looks good.

Depend on what you want you may want to hold off until FL.

Comment 7

[John Lu [:mnjul][Please NI or I'll miss]]

Final TBPL run after rebasing and before landing: https://tbpl.mozilla.org/?rev=321d37228d049c9749d6c907e672f5ff7db28918&tree=Gaia-Try

Comment 8

[John Lu [:mnjul][Please NI or I'll miss]]

Master: https://github.com/mozilla-b2g/gaia/commit/447c3c80150e4e1c93d459933b73d753d8980cb6

The last applicable TBPL run was https://tbpl.mozilla.org/?rev=c167560c4a2bbc69b80f022ac403620d5b4e851a&tree=Gaia-Try and not the one in comment 7.