930402 - [e.me] Access to current keyboard settings with mozSettings

Status: RESOLVED FIXED

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

Description

[Amir Nissim (:amirn)]

E.me uses 'keyboard.current' to detect the keyboard's language.

This API will likely change in 1.2 when 3rd party keyboards are integrated into the OS.

Comment 1

[Preeti Raghunath(:Preeti)]

Tim,

Please review since it is a keyboard issue. Sounds like a feature request.

Comment 2

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

There is no native Web API available for detecting current keyboard locale, and (ab)using settings database as mentioned should not be supported. E.me should simply work against |navigator.language| for now.

That said, you are very welcome to propose APIs for the web platform like |navigator.inputLocale| in dev-webapi.

Comment 3

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

Arthur, could you explain how keyboard.current gets update now?

Comment 4

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

In the past, we have only one keyboard app, which will maintain "keyboard.current" settings entry to the language of the current keyboard.

With 3rd-party Keyboard, we can still set keyboard.current entry (in Gaia system keyboard_manager), but its format will be something like:

{
  manifestURL: "app://keyboard.gaiamobile.org/manifest.webapp",  // This is the manifest URL for an app, might be 3rd-party app here
  inputMethodId: "en"            // Since a keyboard app can include multiple input methods, this is the ID for the active one
}

But I don't think this makes sense to other 3rd-party service, like e.me.
And this is not implemented yet.

Comment 5

[Jason Smith [:jsmith]]

Tim - Can your team triage this?

Comment 6

[Preeti Raghunath(:Preeti)]

moving to 1.3, not a blocker for 1.2 per moving e.me to 1.3

Comment 7

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

This is not a feature of the team.

Comment 8

[Ran Ben Aharon [:ranbena] EverythingMe]

The keyboard language is an important signal for the e.me engine.
1.3 is about to close soon and we shouldn't let this linger. How can we progress?

Comment 9

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

(In reply to Rudy Lu [:rudyl] from comment #4)
> In the past, we have only one keyboard app, which will maintain
> "keyboard.current" settings entry to the language of the current keyboard.
> 
> With 3rd-party Keyboard, we can still set keyboard.current entry (in Gaia
> system keyboard_manager), but its format will be something like:
> 
> {
>   manifestURL: "app://keyboard.gaiamobile.org/manifest.webapp",  // This is
> the manifest URL for an app, might be 3rd-party app here
>   inputMethodId: "en"            // Since a keyboard app can include
> multiple input methods, this is the ID for the active one
> }
> 
> But I don't think this makes sense to other 3rd-party service, like e.me.
> And this is not implemented yet.

Please don't do that. It doesn't make sense for the web, nor even certified app-only (e.me).

(In reply to Ran Ben Aharon (Everything.me) from comment #8)
> The keyboard language is an important signal for the e.me engine.
> 1.3 is about to close soon and we shouldn't let this linger. How can we
> progress?

As I said in comment 2, E.me should use |navigator.language|, which is the UI language of the phone. Anything else is not a information keyboard service could produce; even if it could it would not be reliable (think of a handwriting recognition "keyboard" that supports 5 languages in 3 scripts -- it's impossible to tell the language of the output).

Comment 10

[Ran Ben Aharon [:ranbena] EverythingMe]

I understand. Can we assume that the standard keyboards will set keyboard.current while 3rd party keyboards will not? If so, can we request that the value would be undefined/'' in that case?

Comment 11

[Jason Smith [:jsmith]]

3rd party keyboards is preffed off right now on 1.2 & master & is a targeted feature, so this does not block the 1.3 release.

Comment 12

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #11)
> 3rd party keyboards is preffed off right now on 1.2 & master & is a targeted
> feature, so this does not block the 1.3 release.

One word fix - only 1.2, but master it's on. But we were planning to ship w/o this anyways, so we don't need to block on this.

Comment 13

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

(In reply to Jason Smith [:jsmith] from comment #11)
> 3rd party keyboards is preffed off right now on 1.2 & master & is a targeted
> feature, so this does not block the 1.3 release.

But the settings key e.me is rely on has been moved already.

(In reply to Ran Ben Aharon (Everything.me) from comment #10)
> I understand. Can we assume that the standard keyboards will set
> keyboard.current while 3rd party keyboards will not? If so, can we request
> that the value would be undefined/'' in that case?

Sorry for the late reply but this still sounds like an workaround to me. Couldn't e.me get the UI language and determine the language of the search string on the server side?

Comment 14

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

ni? djf as he is working on the new keyboard app.

Comment 15

[Jason Smith [:jsmith]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #13)
> (In reply to Jason Smith [:jsmith] from comment #11)
> > 3rd party keyboards is preffed off right now on 1.2 & master & is a targeted
> > feature, so this does not block the 1.3 release.
> 
> But the settings key e.me is rely on has been moved already.

That's not really relevant to the bug here. I still don't see why this would become a blocker for 1.3 if this was already a non-blocker 1.2, which was when we were originally intending to ship third party keyboards.

Comment 16

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

(In reply to Jason Smith [:jsmith] from comment #15)
> That's not really relevant to the bug here. I still don't see why this would
> become a blocker for 1.3 if this was already a non-blocker 1.2, which was
> when we were originally intending to ship third party keyboards.

You should probably ask Preeti on that per comment 6.

Comment 17

[Ran Ben Aharon [:ranbena] EverythingMe]

Tim, the UI language is irrelevant most of the time. Many users prefer an English UI but search for local content.
We're looking into determining query language server side, but I would rather it be a fallback when keyboard.current isn't set.
So I would still like to know - what would be the value of keyboard.current when a 3rd party keyboard is used?

Comment 18

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

(In reply to Ran Ben Aharon (Everything.me) from comment #17)
> Tim, the UI language is irrelevant most of the time. Many users prefer an
> English UI but search for local content.
> We're looking into determining query language server side, but I would
> rather it be a fallback when keyboard.current isn't set.
> So I would still like to know - what would be the value of keyboard.current
> when a 3rd party keyboard is used?

For 1.3 we could restore that in the current keyboard codebase, given the fact it's really needed in e.me. It would be really helpful if developers for e.me could write the patch and submit for review.

Beyond 1.3 since the keyboard app will be rewritten by djf, I will ask him to weight in on that.

Abusing mozSettings is never a good resolution -- it would be better if you raise your point in webapi mailing list for a discussion on this use case.

Comment 19

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

Taking. I will also provide some clean-up on the built-in keyboard start-up code.

Comment 20

[Jason Smith [:jsmith]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #19)
> Taking. I will also provide some clean-up on the built-in keyboard start-up
> code.

Exactly why did we just morph a bug here? This is literally confusing for anyone who needs to triage this.

Comment 21

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

(In reply to Jason Smith [:jsmith] from comment #20)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> comment #19)
> > Taking. I will also provide some clean-up on the built-in keyboard start-up
> > code.
> 
> Exactly why did we just morph a bug here? This is literally confusing for
> anyone who needs to triage this.

Rephrasing comment 18 by copy-paste bug 942790 comment 20:

I provided an explanation on bug 930402 comment 9 asking e.me to move away from relying on keyboard.current mozSetting value. Given the fact Ran decline to do that, I morph the bug in bug 930402 comment 18 so that built-in keyboard could continue provide the information to e.me legacy code.

I have my patch ready and will send it in hours.

Comment 22

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

Switching components since the bug has been morphed.

Comment 23

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:master PR#14105

Comment 24

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

Ran,

Even if my patch reaches master (v1.3), v1.2 keyboard will still not update keyboard.current for you. If you really need that in v1.2 please raise this bug to koi?/+ and I think I could provide a safe, band-aid patch.

What do you think?

Also, please make sure e.me moving away relying on keyboard.current value in the long run, as I stated in comment 2, and again in comment 8. Thanks.

Comment 25

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

Comment on attachment 8339107 [details] [review]
mozilla-b2g:master PR#14105

r+ with one question commented on the bug.

BTW, why would we want to set 'keyboard.current' back to 'undefined'?

--
Thanks for taking this bug and also helped clean up some keyboard app code.

Comment 26

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

master: https://github.com/mozilla-b2g/gaia/commit/5164b8141fff0c617ec7ef51ac34c623482e4a10

(In reply to Rudy Lu [:rudyl] from comment #25)
> BTW, why would we want to set 'keyboard.current' back to 'undefined'?

That was what Ran asked for in comment 10.

Comment 27

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

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=4a1e8e0f1d4a

I was unaware of Gaia UI tests is failing until I merge the patch....

Comment 28

[Ran Ben Aharon [:ranbena] EverythingMe]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #24)
> Even if my patch reaches master (v1.3), v1.2 keyboard will still not update
> keyboard.current for you. If you really need that in v1.2 please raise this
> bug to koi?/+ and I think I could provide a safe, band-aid patch.

The problem is that on every search we send our server the keyboard lang in order to get relevant results and for 3rd party keyboards, we'd send the wrong language and potentially mess up results :/ In this case I'd rather send no keyboard language at all. In order to do that, I'm gonna have to determine that a 3rd party keyboard is being currently used.
 
> Also, please make sure e.me moving away relying on keyboard.current value in
> the long run, as I stated in comment 2, and again in comment 8. Thanks.

Our server guys pointed out that string language detection is tricky in some languages so it's best not to rely on it.

Comment 29

[Ran Ben Aharon [:ranbena] EverythingMe]

So I'm basically saying, is there a way to determine that a 3rd party keyboard is currently being used?

Comment 30

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

(In reply to Ran Ben Aharon (Everything.me) from comment #29)
> So I'm basically saying, is there a way to determine that a 3rd party
> keyboard is currently being used?

After this bug has landed, you could just do your usual thing in v1.3 -- read keyboard.current -- if it's "undefined" the user is running a 3rd-party keyboard.

For v1.4 I am waiting for :djf's comment.

You haven't answer my question in comment 24 -- do you need this bug to be landed in v1.2? If not for v1.2 the value of keyboard.current will be "en" regardless of any layout.

Comment 31

[Ran Ben Aharon [:ranbena] EverythingMe]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #30)
> After this bug has landed, you could just do your usual thing in v1.3 --
> read keyboard.current -- if it's "undefined" the user is running a 3rd-party
> keyboard.

Perfect

> For v1.4 I am waiting for :djf's comment.
> 
> You haven't answer my question in comment 24 -- do you need this bug to be
> landed in v1.2? If not for v1.2 the value of keyboard.current will be "en"
> regardless of any layout.

Yes, please land it in v1.2

Thanks, Tim!!

Comment 32

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

(In reply to Ran Ben Aharon (Everything.me) from comment #31) 
> > You haven't answer my question in comment 24 -- do you need this bug to be
> > landed in v1.2? If not for v1.2 the value of keyboard.current will be "en"
> > regardless of any layout.
> 
> Yes, please land it in v1.2
> 
> Thanks, Tim!!

Comment 33

[Preeti Raghunath(:Preeti)]

Tim

Can you please provide context on why this needs to be a blocker?

User impact if not taken in 1.2

Comment 34

[Ran Ben Aharon [:ranbena] EverythingMe]

Basically, E.me search requests send out keyboard language the server, in order to return better results - apps and autocomplete suggestions alike. We harvest the keyboard language value from Settings (our only option at this point).

Third party keyboards do not change this value, therefore misleading the E.me server and potentially leading to irrelevant result.

This patch simply sets the keyboard language value to "undefined" when third party keyboards are chosen, therefore letting E.me servers ignore it.

Comment 35

[Jason Smith [:jsmith]]

(In reply to Ran Ben Aharon (Everything.me) from comment #34)
> Basically, E.me search requests send out keyboard language the server, in
> order to return better results - apps and autocomplete suggestions alike. We
> harvest the keyboard language value from Settings (our only option at this
> point).
> 
> Third party keyboards do not change this value, therefore misleading the
> E.me server and potentially leading to irrelevant result.
> 
> This patch simply sets the keyboard language value to "undefined" when third
> party keyboards are chosen, therefore letting E.me servers ignore it.

Hmm...but we've turned off third party keyboards for 1.2 to my understanding.

Does this have user impact outside of third party keyboards?

Comment 36

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

(In reply to Jason Smith [:jsmith] from comment #35)
> Hmm...but we've turned off third party keyboards for 1.2 to my understanding.

You understand it wrong.

Continuing what Ran said in comment 34 and what I had said in comment 13: We remove the feature to set keyboard.current in built-in keyboard during 1.2 development phase because such "hack" around mozSettings shouldn't be supported anymore. Given the fact Ran have made his point on the necessarily of the hack, in comment 18 I agreed to restore the hack and personally wrote a patch for it. That's the patch check-in in this bug. E.me would need the patch, at least the 2nd part of the patch to reach 1.2 so built-in keyboard would continue to supply the information to e.me through this hack.

In the discussion of this bug we have also established a fact that since future 3rd-party keyboards will have no access to this hack, built-in keyboard should change the value to "undefined" when it's being switched away (something not applicable to v1.2 since the feature is disabled).

> Does this have user impact outside of third party keyboards?

Please re-read comment 30 on the behavior of built-in keyboard layouts.

I hope this is the last time I am asked to summarize this bug. I hope my English is not that bad.

Comment 37

[Jason Smith [:jsmith]]

Comment for 12/5 triage:

There's still lack of justification on why is this needed for 1.2. 1.2 blockers need to be critical issues at this point, which I do not see evidence supporting this. I think this is a blocking- unless there's a significant proof of critical end user impact.

Comment 38

[Preeti Raghunath(:Preeti)]

Fabrice,

Request for technical guidance.

Comment 39

[[:fabrice] Fabrice Desré]

Hi All,

I just read all the comments here again, and I think we need to uplift Tim's patch to 1.2 so that ev.me gets the same behavior they used to have. Right now they are broken by the churn we had around 3rd party keyboards. But for 1.4 a better solution needs to happen.

Comment 40

[Jason Smith [:jsmith]]

Here's a summary for triage:

This bug applies a hack that used to be present to ensure that the correct keyboard language is sent to e.me when e.me search occur. The keyboard language is used by an e.me search to help improve e.me search results. In the development of third-party keyboards, this regressed to send the language of "en." That means we've regressed the quality of e.me search results in the 1.2 timeframe. Fixing this bug ensures that we get back the quality of e.me search results we had originally with the correct keyboard language sent to e.me.

Short answer - it's a regression on the quality of e.me search results, which makes this a blocker for release.

Comment 41

[Jason Smith [:jsmith]]

Talked with rel man in person - in agreement to block based on the above comments.

Comment 42

[John Ford [:jhford] CET/CEST Berlin Time]

This bug was partially uplifted.

Uplifted 5164b8141fff0c617ec7ef51ac34c623482e4a10 to:
v1.3 already had this commit

Commit 5164b8141fff0c617ec7ef51ac34c623482e4a10 didn't uplift to branch v1.2

Comment 43

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 5164b8141fff0c617ec7ef51ac34c623482e4a10
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 44

[David Flanagan [:djf]]

Sorry it took me so long to respond to the needinfo on this, Tim!

To make up for it, I was going to uplift the patch for you. But then I saw how big the patch was and got scared off.  It has been a while since I thought about that part of the keyboard, so I think I should leave resolving the merge conflicts to you.

The only thought I have is that if you set keyboard.current from system/js/keyboard_manager.js instead of in the keyboard app then it could work for 3rd party keyboards as well.

Comment 45

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

(In reply to David Flanagan [:djf] from comment #44)
> Sorry it took me so long to respond to the needinfo on this, Tim!
> 
> To make up for it, I was going to uplift the patch for you. But then I saw
> how big the patch was and got scared off.  It has been a while since I
> thought about that part of the keyboard, so I think I should leave resolving
> the merge conflicts to you.

I was only intend to uplift part II of the patch :) Will do that now.

I uplifted https://github.com/timdream/gaia/commit/95e254dc0b5cc046891dd54c012ff1d51e4382cb as
https://github.com/timdream/gaia/commit/4f53ba8b3628ac311253fc28dfdf66e7ba6832de

> The only thought I have is that if you set keyboard.current from
> system/js/keyboard_manager.js instead of in the keyboard app then it could
> work for 3rd party keyboards as well.

This works under the assumption that there is an one-to-one relation to keyboard layout (or "input source") to a given natural language/locale. What if there is voice input program accepts multiple languages? Handwriting? That's why I dislike the idea -- we just kick the problem further without addressing it.

Comment 46

[Amir Nissim (:amirn)]

Tim, Can you suggest how to implement this feature in 1.4? (bug #961675)

Comment 47

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

See bug 961675 comment 2

Comment 48

[Kevin Grandon :kgrandon]

Mass modify - set status-b2g-v2.0 fixed for fixed bugs under vertical homescreen dependency tree.