859201 - [Contacts] The Contacts App Should NOT be using the mozKeyboard API

Status: RESOLVED FIXED

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

Description

[Evan Tseng [:evanxd]]

In the contacts.js file, the goToSelectTag function used the mozKeyboard.removeFocus() API.

See in the Gaia master:
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/contacts.js#L403

We thought the Contacts App should not use the mozKeyboard API.

Comment 1

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

Can we replace this with document.activeElement.blur() instead?

mozKeyboard API has security implication too; we should remove this permission from the production phone.

Comment 2

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

I am sorry I needinfo the wrong person :-/

Comment 3

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

Now this is the right set of the person to CC to.

Comment 4

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

Stealing, I have verified my comment 1 works with STR in bug 833231 comment 0.

Comment 5

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

Attachment: Github: https://github.com/mozilla-b2g/gaia/pull/9248

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

Thanks Tim!

Comment 7

[Alex Keybl [:akeybl]]

Not blocking for now, doesn't seem user critical.

Tim - can you renominate with justification? If it's a security issue, please make sure to sync up with Paul and make sure he agrees this is critical to fix for v1.0.1. We'd rather not take a change here.

Comment 8

[Paul Theriault [:pauljt] (no longer reading bugmail)]

This seems like a pretty simple change for a big risk gain here. Its not directly exploitable, but given the contacts app has a relatively large attack surface with the facebook integration etc (as opposed to the keybaord app which is completely local) I think this is worth fixing. Especially given how simple a change this is.

Comment 9

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Er, first sentence should read: retty simple change for a big risk MITIGATION...

Comment 10

[Jose Manuel Cantera]

(In reply to Paul Theriault [:pauljt] from comment #8)
> This seems like a pretty simple change for a big risk gain here. Its not
> directly exploitable, but given the contacts app has a relatively large
> attack surface with the facebook integration etc (as opposed to the keybaord
> app which is completely local) I think this is worth fixing. Especially
> given how simple a change this is.

I agree, pretty simple change for a big win

Comment 11

[alberto.pastor]

Comment on attachment 738579 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/9248

\o/ 
Tests passing locally.
Thanks!

Comment 12

[Alex Keybl [:akeybl]]

Is v1.0.1 even affected? Bug 855175 appears to have only landed to v1.1. If that's correct, please leo? instead

Comment 13

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

Right, bug 833231 only lands on v1-train.

Comment 14

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

master: https://github.com/mozilla-b2g/gaia/commit/48415d26d6821bdadd4a43d7ccc2a87672cc0bdf

Comment 15

[James Lal [:lightsofapollo]]

Uplifted 48415d26d6821bdadd4a43d7ccc2a87672cc0bdf to:
v1-train: 7138457c3ff9461b531f07e2c956c0129f962eb6