[Settings][V1.2] The settings for third party keyboards will still display after uninstalling the keyboard

RESOLVED FIXED

Status

Firefox OS
Gaia::Settings
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: whsu, Assigned: gnarf)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
arthurcc
: review+
Details | Review | Splinter Review
(Reporter)

Description

5 years ago
* Description:
  This problem happens on v1.2 build(M-C).
  If user uninstalls an in used third party keyboard, user reinstalls the third party keyboard.
  User will see the previous settings. The setting values shouldn't be kept.

  You can refer to the following online video to know the bug.
  - https://dc1.safesync.com/LMrHwTtl/WP_20131031_004.mp4?a=-fGXpw1JAaM

* Reproduction steps:
  1. Install a third party keyboard. You can use following fake app to do the test.
     - http://mpizza.github.io/ime/install.html
  2. Enable the third party keyboard
  3. Launch the contact app to use it
  4. Press the home button to go to homescreen.
  5. Uninstall the third party keyboard.
  6. Install the third party keyboard without enabling any virtual keyboard.
  7. Go to keyboard settings to check the enabling keyboard.

* Expected result:
  No third party keyboard is enabled

* Actual result:
  The previous setting is kept

* Reproduction build:(V1.2)
 - Gaia:     a788ea1a3e04716938bd41a559c36a831cf1190b
 - Gecko:    http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/4fbe1b01ed63
 - BuildID   20131030004003
 - Version   26.0a2
(Reporter)

Updated

5 years ago
blocking-b2g: --- → koi?
Whiteboard: burirun3
Hi Corey,
   Can you help to take a look?
   Thanks.
Flags: needinfo?(gnarf37)
(Assignee)

Comment 2

5 years ago
Now that bug 912010 has landed on v1.2, can you confirm this still reproduces?  I think I already fixed this problem in that bugs commit.
Flags: needinfo?(gnarf37) → needinfo?(whsu)
(Reporter)

Comment 3

5 years ago
Hi, Corey,

I still can reproduce this bug on following build. (/_\)
* Test Build:
 - Gaia:     e717aec947571f5daf923c040a82f9f0719bb526
 - Gecko:    http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/54de309e18a9
 - BuildID   20131101004000
 - Version   26.0
Flags: needinfo?(whsu)
(Assignee)

Updated

5 years ago
Assignee: nobody → gnarf37
Status: NEW → ASSIGNED
(Assignee)

Comment 4

5 years ago
How exactly am I supposed to install this app.  I figured I should just navigate to it in the browser but this is the result: http://cl.ly/image/26080B2R1E0a
(Assignee)

Comment 5

5 years ago
nevermind... terrible error message means i forgot to enable data after a reset-gaia
(Assignee)

Comment 6

5 years ago
Created attachment 827033 [details] [review]
github PR

Not quite ready for review (no units yet, and still a big question open about the settings stuff from my side) but I want feedback from kaze to help guide me the rest of the way.
Attachment #827033 - Flags: feedback?(kaze)
(Assignee)

Updated

5 years ago
Component: Gaia::Keyboard → Gaia::Settings
(Assignee)

Comment 7

5 years ago
Updated the title, you can actually remove step 6 and this gets even weirder...

If you have opened the "Keyboards" panel in settings, then uninstall the third party keyboard, the enabled keyboard list still includes the keyboards from the third party app.  Conversely, if you add a new keyboard and have already opened the add more keyboards panel, you will never see the layouts provided by the new keyboard.
Summary: [Keyboard][V1.2] The settings value of third party keyboard will be kept if uninstalling the in used keyboard → [Settings][V1.2] The settings for third party keyboards will still display after uninstalling the keyboard
(Reporter)

Comment 8

5 years ago
Hi, Corey,

I don't think so.
My bad. I don't describe this bug clearly.

No matter whether I opened the "Keyboards" panel in settings or not.
After I uninstall the third party keyboard then re-install it again, you will see the third party keyboards are enabled as previous.
As I know, if user uninstall an app, these settings should not be kept.

You can refer to the following online video to know my test steps. Thank you! :)
- https://dc1.safesync.com/LMrHwTtl/WP_20131105_002.mp4?a=1-mPE5zLgCw
(Assignee)

Comment 9

5 years ago
Nope, your steps to reproduce were just fine.  It's the same root cause.  The keyboard helper will actually do the right thing, but the settings app doesn't get the right events.

Even after I wire up the right events, I discovered that the actual list of ALL layouts (with checkboxes to enable) never adds or removes new apps as they are installed/removed.

Can you test my branch in the pull request to see if it fixes your problem?
(Reporter)

Comment 10

5 years ago
Hi, Corey,

I don't prefer to test a local build since it will cause double effort.
If the patch is ready (reviewed and merged), I will grab a official build to test it (PVT build).
Many thanks!
(Assignee)

Comment 11

5 years ago
Well, I've tested it on my development branch using your STR and can't reproduce, so pretty sure I've got it nailed down.

It will probably be another day or three before this sees review/resolved status.  I only asked for you to test it from my development branch because you seemed unsure of your own description of the bug.  I'll be happy to land a fix for the problem I found while trying to reproduce this anyway! :)
Comment on attachment 827033 [details] [review]
github PR

Arthur will be much more helpful for these ListView-related questions. Redirecting the f?. Looks like a `destroy` method could help.
Attachment #827033 - Flags: feedback?(kaze) → feedback?(arthur.chen)
Comment on attachment 827033 [details] [review]
github PR

Please check my comments in github, thanks.
Attachment #827033 - Flags: feedback?(arthur.chen) → feedback-
(Assignee)

Updated

5 years ago
Depends on: 935710
:kaze can you help understand if this is a valid userflow when using third party keyboard ? Traige was unclear on that hence we do not know if this is a blocker
Flags: needinfo?(kaze)
(Assignee)

Comment 15

5 years ago
This is definitely valid user flow:

* go to keyboards settings, add more layouts page
* install a keyboard, choose none of the layouts
* go back to keyboards settings to turn on layouts

Expected: the new application keyboards are available to be enabled

Actual: the new keyboard application does not show up in the settings app until you kill the app and restart it.

---

The same bug shows up when removing a keyboard (the old keyboard layouts are still in the settings app) as well as when removing and installing the keyboard (the old keyboard settings are still enabled, even though you didn't re-enable any keyboards upon installing again)
(Assignee)

Updated

5 years ago
Depends on: 936631
(Assignee)

Updated

5 years ago
Depends on: 936643
koi+ based on comment 0.
blocking-b2g: koi? → koi+
Whiteboard: burirun3 → burirun3, [3rd-party-keyboard]
It sounds like this bug takes more than the time we have in v1.2 to resolve (or, it had used up all the time available to resolve koi+).

This bug is recoverable by kill and relaunch the Setting app. Given it's relative small impact (compare to work needed) I decided to remove koi+ this bug and it's dependencies. 

Please seek approval after you fix this bug.
blocking-b2g: koi+ → ---
blocking-b2g: --- → 1.3+
(Assignee)

Comment 18

5 years ago
clearing ni? kaze - question from comment 14 has been answered
Flags: needinfo?(kaze)
(Assignee)

Comment 19

5 years ago
Comment on attachment 827033 [details] [review]
github PR

PR Updated with latest.  Won't check in on this one until later
Attachment #827033 - Flags: feedback- → review?(arthur.chen)
(Assignee)

Comment 20

5 years ago
Is there a more up to date with current manifest changes version of the test keyboard app I can install/remove for testing?
Flags: needinfo?(whsu)
(Reporter)

Comment 21

5 years ago
Hi, Gary,

Could I have your help?
Because of the some patches were merged, we cannot install the third party keyboard app you developed now.
Do there have any workaround to solve this problem?
Thanks!
Flags: needinfo?(whsu) → needinfo?(gchen)
Hi, William,
    Okay, I'll put this task on my queue.
Flags: needinfo?(gchen)
Comment on attachment 827033 [details] [review]
github PR

Please check my comments in github about the `_observeAndCall` function.
Attachment #827033 - Flags: review?(arthur.chen)
See https://bugzilla.mozilla.org/show_bug.cgi?id=942790#c17.
blocking-b2g: 1.3+ → 1.3?
(Assignee)

Comment 25

5 years ago
Comment on attachment 827033 [details] [review]
github PR

Moved the observeAndCall into a helper passed as a third argument to the template
Attachment #827033 - Flags: review?(arthur.chen)
(Assignee)

Updated

5 years ago
Blocks: 936638
Comment on attachment 827033 [details] [review]
github PR

r=me. Thank you Corey! As we are going to branch for v1.3, please also ni? Fabrice if you would like to land this before branching.
Attachment #827033 - Flags: review?(arthur.chen) → review+
Moving to 1.4? per offline discussion - we'll be keeping this preffed off for 1.3.
blocking-b2g: 1.3? → 1.4?
(Assignee)

Comment 28

5 years ago
squashed and merged, tree is closed so setting checkin-neeeded
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/9e3468cf23d2be02a3689e25ce19dd3db3792903
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
blocking-b2g: 1.4? → ---
You need to log in before you can comment on or make changes to this bug.