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)
Tracking
(blocking-b2g:koi+, 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.
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Comment 1•11 years ago
|
||
(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
Assignee | ||
Comment 2•11 years ago
|
||
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
Updated•11 years ago
|
Assignee: timdream → nobody
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → timdream
Summary: Backout or Pref Off 3rd Party Keyboard Support for 1.2 → Pref Off 3rd Party Keyboard Support for 1.2
Assignee | ||
Comment 5•11 years ago
|
||
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)
Assignee | ||
Comment 6•11 years ago
|
||
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 7•11 years ago
|
||
Comment on attachment 8336650 [details]
mozilla-b2g:master PR#13945
Looks good.
Thanks.
Attachment #8336650 -
Flags: review?(rlu) → review+
Comment 8•11 years ago
|
||
Comment on attachment 8336653 [details]
mozilla-b2g:v1.2 PR#13946
r=me. Thank you.
Attachment #8336653 -
Flags: review?(rlu) → review+
Assignee | ||
Comment 9•11 years ago
|
||
Comment 10•11 years ago
|
||
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 → ---
Assignee | ||
Comment 11•11 years ago
|
||
Assignee | ||
Comment 12•11 years ago
|
||
Assignee | ||
Comment 13•11 years ago
|
||
(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 ago → 11 years ago
Resolution: --- → FIXED
Comment 15•11 years ago
|
||
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.
Comment 16•11 years ago
|
||
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
Assignee | ||
Comment 18•11 years ago
|
||
(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)
Assignee | ||
Comment 19•11 years ago
|
||
master revert: 2cc78d696482e0434b584f5645af55e3105e59a2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 20•11 years ago
|
||
(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?
Assignee | ||
Comment 21•11 years ago
|
||
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.
Assignee | ||
Comment 22•11 years ago
|
||
(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)
Reporter | ||
Comment 23•11 years ago
|
||
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
Assignee | ||
Comment 24•11 years ago
|
||
v1.2 revert: 5a0802f44565ac9a17f5a720259ac2689ab67ff7
I have other concern with my own patch. Will submit a patch for re-review and reland.
Assignee | ||
Comment 27•11 years ago
|
||
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)
Assignee | ||
Comment 28•11 years ago
|
||
Comment on attachment 8337223 [details]
mozilla-b2g:v1.2 PR#13968
v1.2 patch with updated unit tests.
Attachment #8337223 -
Flags: review?(rlu)
Assignee | ||
Updated•11 years ago
|
Attachment #8336650 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #8336653 -
Attachment is obsolete: true
Comment 29•11 years ago
|
||
Comment on attachment 8337222 [details]
mozilla-b2g:master PR#13967
r=me.
Thanks.
Attachment #8337222 -
Flags: review?(rlu) → review+
Comment 30•11 years ago
|
||
Comment on attachment 8337223 [details]
mozilla-b2g:v1.2 PR#13968
r+ as well.
Attachment #8337223 -
Flags: review?(rlu) → review+
Comment 31•11 years ago
|
||
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?
Assignee | ||
Comment 32•11 years ago
|
||
(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.
Assignee | ||
Comment 33•11 years ago
|
||
master: https://github.com/mozilla-b2g/gaia/commit/9531af422d45206b6b5a5f7e733a0047439784a4
v1.2: https://github.com/mozilla-b2g/gaia/commit/6e66cb790d5b931d7cfb3256b8cb90a4471c1a3c
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 34•11 years ago
|
||
Tagging verifyme per request in comment 23.
Flags: needinfo?(whsu)
Keywords: verifyme
Comment 35•11 years ago
|
||
(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).
Assignee | ||
Comment 36•11 years ago
|
||
(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.
Comment 37•11 years ago
|
||
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)
Comment 38•11 years ago
|
||
--------- Keep Needinfo myself to remind myself ---------
Flags: needinfo?(whsu)
Comment 39•11 years ago
|
||
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)
Updated•11 years ago
|
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 40•11 years ago
|
||
Can you open a followup bug for the item you've found?
Keywords: verifyme
Assignee | ||
Comment 41•11 years ago
|
||
(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)
Comment 42•11 years ago
|
||
Hi, Tim,
Thanks for the reminder.
I submitted the bug - Bug 944295
Flags: needinfo?(whsu)
Assignee | ||
Updated•11 years ago
|
Status: VERIFIED → RESOLVED
Closed: 11 years ago → 11 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.
Description
•