Closed Bug 941885 Opened 11 years ago Closed 11 years ago

Pref Off 3rd Party Keyboard Support on 1.2 and master

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:koi+, b2g-v1.2 fixed)

RESOLVED FIXED
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed

People

(Reporter: jsmith, Assigned: timdream)

References

Details

(Whiteboard: [3rd-party-keyboard])

Attachments

(3 files, 2 obsolete files)

Per a discussion with Fabrice & Andreas, 3rd party keyboard needs to be turned off for 1.3, whether that means backed out or preffed off.
blocking-b2g: --- → koi+
Whiteboard: [3rd-party-keyboard]
(In reply to Jason Smith [:jsmith] from comment #0) > Per a discussion with Fabrice & Andreas, 3rd party keyboard needs to be > turned off for 1.3, whether that means backed out or preffed off. Meant to say - turned off for 1.2
With little information on this side of the ocean, I could try to figure out what's the requirement here based on discussion happen in this office. It's too risky to backout anything -- disable in v1.2 is the only available option here. However, we would have to keep it enabled in master so that we could begin testing and identify the gap between now and production. We could always disable it again in v1.3 branch when it branches out. I will come up with a patch again later today.
Assignee: nobody → timdream
Assignee: timdream → nobody
Assignee: nobody → timdream
Summary: Backout or Pref Off 3rd Party Keyboard Support for 1.2 → Pref Off 3rd Party Keyboard Support for 1.2
Comment on attachment 8336650 [details] mozilla-b2g:master PR#13945 After looking up app_install_manager.js, I realized I cannot prevent an input packaged app from being installed by just looking at it's updateManifest, since it does not contain information on it's type/role. So I ended up did the pref in keyboard_help.js. When 'keyboard.3rd-party-app.enabled' is set to true, keyboard apps will be run out-of-process, and UI for selecting layouts other than Gaia keyboard apps will be shown. When set to false, keyboard apps will be run in-process and UI will not show any non-Gaia keyboard apps (so it's impossible launch any of them even if you manage to install it). Developer option is being removed too so the value doesn't get flipped during runtime. Only developers with root access will be able to flip the value. The option is set to true in master, and false in v1.2 branch.
Attachment #8336650 - Flags: review?(rlu)
Comment on attachment 8336653 [details] mozilla-b2g:v1.2 PR#13946 This is the same patch commit on v1.2 with additional commit to turn the pref off.
Attachment #8336653 - Flags: review?(rlu)
Comment on attachment 8336650 [details] mozilla-b2g:master PR#13945 Looks good. Thanks.
Attachment #8336650 - Flags: review?(rlu) → review+
Comment on attachment 8336653 [details] mozilla-b2g:v1.2 PR#13946 r=me. Thank you.
Attachment #8336653 - Flags: review?(rlu) → review+
This has broken the keyboard (for me at least) on M-C + Master. The keyboard comes up over the page (hides it) and whenever a key is touched the keyboard hides again... and the key isn't send to the form. Rolling back this commit fixes it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #10) > This has broken the keyboard (for me at least) on M-C + Master. The keyboard > comes up over the page (hides it) and whenever a key is touched the keyboard > hides again... and the key isn't send to the form. Rolling back this commit > fixes it. You need to update Gecko to include bug 847763, or simply disable keyboard oop on master build by flipping the settings entry.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Attached image 2013-11-22-15-17-58.png (deleted) —
This patch has caused a large white box to appear above the keyboard. The white box is perfectly sized to obscure the whole of the remaining app area (see screenshot). Everything is still functional, but the main app space is hidden by this box. I think we should back this patch out.
For above comment # 15: Base: V1.2_US_20131115.cfg Gaia: 13687c598c3b5ef4cff9eef8d0ed3bb57c5f9d65 Gecko: http://hg.mozilla.org/mozilla-central/rev/dbf94e314cde BuildID 20131122040202 Version 28.0a1 Device Hamachi
John - Can you back this out?
Flags: needinfo?(jhford)
(In reply to Jason Smith [:jsmith] from comment #17) > John - Can you back this out? I am backing out the master patch myself. Sorry for not checking B2G/Desktop first.
Flags: needinfo?(jhford)
master revert: 2cc78d696482e0434b584f5645af55e3105e59a2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #19) > master revert: 2cc78d696482e0434b584f5645af55e3105e59a2 Does this also need to be reverted on 1.2 as well?
To prevent this bug popping up in koi+ query, if the breakage doesn't get resolved in days I will re-land but disable the patch on master, and figure out what it takes to enable it on another bug. Monday.
(In reply to Jason Smith [:jsmith] from comment #20) > (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from > comment #19) > > master revert: 2cc78d696482e0434b584f5645af55e3105e59a2 > > Does this also need to be reverted on 1.2 as well? No. v1.2 is fine. It's landed and disabled (unless for some strange reason people found the same problem in v1.2 branch)
William - Can you do some negative testing here to see what happens in this patch to pref off 3rd party keyboards?
Flags: needinfo?(whsu)
Keywords: verifyme
v1.2 revert: 5a0802f44565ac9a17f5a720259ac2689ab67ff7 I have other concern with my own patch. Will submit a patch for re-review and reland.
Flags: needinfo?(whsu)
Keywords: verifyme
Comment on attachment 8337222 [details] mozilla-b2g:master PR#13967 This patch will now disable the feature one master branch too until we found what is going on in comment 15.
Attachment #8337222 - Flags: review?(rlu)
Comment on attachment 8337223 [details] mozilla-b2g:v1.2 PR#13968 v1.2 patch with updated unit tests.
Attachment #8337223 - Flags: review?(rlu)
Attachment #8336650 - Attachment is obsolete: true
Attachment #8336653 - Attachment is obsolete: true
Comment on attachment 8337222 [details] mozilla-b2g:master PR#13967 r=me. Thanks.
Attachment #8337222 - Flags: review?(rlu) → review+
Comment on attachment 8337223 [details] mozilla-b2g:v1.2 PR#13968 r+ as well.
Attachment #8337223 - Flags: review?(rlu) → review+
Whats the plan for 1.3? Do we have a list of bugs we need to have confidence to let this ride the 1.3 train?
(In reply to Andreas Gal :gal from comment #31) > Whats the plan for 1.3? Do we have a list of bugs we need to have confidence > to let this ride the 1.3 train? I thought we are not supposed to add feature to v1.3 now? Some improvements has already been working on in bug 835404 and friends -- stability of 3rd-party keyboard app itself would need to be identified this week.
Tagging verifyme per request in comment 23.
Flags: needinfo?(whsu)
Keywords: verifyme
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #32) > (In reply to Andreas Gal :gal from comment #31) > > Whats the plan for 1.3? Do we have a list of bugs we need to have confidence > > to let this ride the 1.3 train? > > I thought we are not supposed to add feature to v1.3 now? > > Some improvements has already been working on in bug 835404 and friends -- > stability of 3rd-party keyboard app itself would need to be identified this > week. Since this missed 1.2, its definitely in scope for 1.3 (with high urgency). If you think you are really close, I am sure we can negotiate with drivers to try for 1.3 (I am pretty sure nobody will fight that). The idea is to not drag this out into the stabilization window (for 1.2 we are 2 1/2 months into that). If you are close we can enable this for 1.3 and see if its in a better shape on that train (seems so). 1.3 stabilization is only about to start. If we see more bugs as we go through 1.3 stabilization, we will face the same decision. If not, its in. Everyone wants this feature in, just not at the expense of blowing up the schedule. Note: We have a external dependency on this work for 1.4, so we can as well try for 1.3, because no latter than 1.4 we will have to block on this. There would be some benefit of getting the chance to test this in the 1.3 cycle (and have people write external keyboards).
(In reply to Andreas Gal :gal from comment #35) > Since this missed 1.2, its definitely in scope for 1.3 (with high urgency). > If you think you are really close, I am sure we can negotiate with drivers > to try for 1.3 (I am pretty sure nobody will fight that). The idea is to not > drag this out into the stabilization window (for 1.2 we are 2 1/2 months > into that). If you are close we can enable this for 1.3 and see if its in a > better shape on that train (seems so). 1.3 stabilization is only about to > start. If we see more bugs as we go through 1.3 stabilization, we will face > the same decision. If not, its in. Everyone wants this feature in, just not > at the expense of blowing up the schedule. > > Note: We have a external dependency on this work for 1.4, so we can as well > try for 1.3, because no latter than 1.4 we will have to block on this. There > would be some benefit of getting the chance to test this in the 1.3 cycle > (and have people write external keyboards). We are working on evaluating feasibility of the feature for v1.3, start by finding the cause of comment 15. It looks like we hit a 0-day(?) regression unrelated to keyboard. I've filed bug 942788 on that. Bug 942790 filed to re-enable keyboard oop (and 3rd-party keyboard support in general) once the said bug is resolved.
Hi, Tim, My bad. Too many trivial things occupied my resource so late replied you. ----------------------------------------------------------------------- I did the following tests. 1. Installation test of third party apps ( Side effect checking ) => Pass 2. Installation test of third party keyboard => Fail. I can install third party via app manager and it works on FxOS. 3. Process monitoring of third party keyboard (* This test depends on second test) => Fail. I can see a process is initialized * Test Result: I didn't see any side effect on third party "app". But, while I used App Manager to do the test, I found (1) The third party keyboard can be installed. (2) I can see the third party on the keyboard settings page (3) The third party keyboard still works on FxOS. So, we need to handle this case and prevent user to install third party keyboard via app manager. Anyway, I will discuss this problem with You and Rudy later. * Test Build: - Gaia: 92cd11ea023dd6598d82d859ae3c945ff6589ce6 - Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/14e91ab12441 - BuildID 20131127004001 - Version 26.0 By the way, I can see the keyboard process was created on V1.3 build. I will start to test OOP feature on V1.3 build. william@tpemozilla:~/Workspace_B2G $ adb shell ps | grep b2g root 109 1 168312 63508 ffffffff 4013d4e0 S /system/b2g/b2g app_375 375 109 78760 29332 ffffffff 400ee4e0 S /system/b2g/plugin-container <System> app_950 950 109 74728 27592 ffffffff 401244e0 S /system/b2g/plugin-container app_1013 1013 109 82424 28320 ffffffff 400124e0 S /system/b2g/plugin-container app_1051 1051 109 69536 25380 ffffffff 400794e0 S /system/b2g/plugin-container <Message app> app_1086 1086 109 82072 32488 ffffffff 400964e0 S /system/b2g/plugin-container <Keyboard process> root 1090 109 66344 22648 ffffffff 400e24e0 S /system/b2g/plugin-container Many thanks. Have a nice day!
Flags: needinfo?(whsu)
--------- Keep Needinfo myself to remind myself ---------
Flags: needinfo?(whsu)
Hi, Tim, Thanks for aggressively handle this case. Per we discussed test result this morning, the test results were impacted by preloaded keyboard app and test environment because I flashed the engineering build. So, I retested the it. 1. Installation test of third party apps ( Side effect checking ) => Pass 2. Installation test of third party keyboard => Pass. But, as we discussed, the installation UI is easy to confuse user. 3. Process monitoring of third party keyboard (* This test depends on second test) => Pass. <Process> APPLICATION USER PID PPID VSIZE RSS WCHAN PC NAME b2g root 136 1 198244 72872 ffffffff 4010c604 S /system/b2g/b2g Homescreen app_441 441 136 73472 31360 ffffffff 40021604 S /system/b2g/plugin-container Usage app_479 479 136 66296 26636 ffffffff 4012c604 S /system/b2g/plugin-container Settings app_680 680 136 72576 31424 ffffffff 400df604 S /system/b2g/plugin-container Messages app_704 704 136 68476 28884 ffffffff 40115604 S /system/b2g/plugin-container (Pre-allocated) root 766 136 63108 22732 ffffffff 40029604 S /system/b2g/plugin-container I would like to say the patch work as we expected but still have UI defect. Many thanks!
Flags: needinfo?(whsu)
Status: RESOLVED → VERIFIED
Can you open a followup bug for the item you've found?
Keywords: verifyme
(In reply to Jason Smith [:jsmith] from comment #40) > Can you open a followup bug for the item you've found? Please file the bug on installation UI and assign to me, thanks, i.e. "[v1.2] Installation UI still shown during keyboard app installation".
Flags: needinfo?(whsu)
Hi, Tim, Thanks for the reminder. I submitted the bug - Bug 944295
Flags: needinfo?(whsu)
Status: VERIFIED → RESOLVED
Closed: 11 years ago11 years ago
Summary: Pref Off 3rd Party Keyboard Support for 1.2 → Pref Off 3rd Party Keyboard Support on 1.2 and master
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: