883344 - Only use default facebook app id if is mozilla partner build

Status: VERIFIED FIXED

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

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: Link to the PR

Comment 3

[Jason Smith [:jsmith]]

Regression from a tef+ blocker in bug 882363. If we don't take this on all branches, then we will be unable to test facebook imports in the Contacts app on any non-customized build.

Comment 4

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

Actually, I wonder if we should enable Facebook import only for official build, or for all open source builds?

Comment 5

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

Chris, do you know the answer to comment 4?

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Jason Smith [:jsmith] from comment #3)
> Regression from a tef+ blocker in bug 882363. If we don't take this on all
> branches, then we will be unable to test facebook imports in the Contacts
> app on any non-customized build.

Note that this is tef+ only to ensure that local builds have working Facebook import. Kind of NPOTB

Comment 7

[Alex Keybl [:akeybl]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #4)
> Actually, I wonder if we should enable Facebook import only for official
> build, or for all open source builds?

If QA can test on a nightly, and developers can test locally (when they need to) then I don't think we'll have a strong opinion here.

Comment 8

[Alex Keybl [:akeybl]]

(Fabrice is out right now by the way, which is why I've reassigned)

Comment 9

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

(In reply to Alex Keybl [:akeybl] from comment #8)
> (Fabrice is out right now by the way, which is why I've reassigned)

I'll love to get comment 4 answered before landing the fix (or, taking over the bug)

Comment 10

[Chris Lee [:clee]]

I will check with our Legal team and respond back.  Thanks.

Comment 11

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

Sorry about keeping people in the dark. In another thread, Chris suggests
the API client_id shouldn't exist in open source build and it's OEM's
responsibility to test the build with the key they included.

I will go ahead and close that bug as invalid.

Comment 12

[Jason Smith [:jsmith]]

That bug resolution isn't going to work. QA needs the ability to test Facebook import functionality on non-customized builds. TEF QA is the primary owner of testing this feature, but Mozilla QA often helps out with testing this feature as well. Without having a capability to this feature, we will not be able to investigate bugs that come in this area and conduct test runs until we have partner customized builds with the Facebook API key included. We need a solution here that allows testing of that functionality on a non-customized build. So I don't think that bug should be resolved invalid - we need an actual solution that allows testing on a non-customized build.

Comment 13

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

Per reply from Chris, we would enable the key behind the official branding build flag, i.e. the key would exist in the public repo, but it would only be enabled when the branding is switched on.

Comment 14

[[e-mail]]

take it.

Comment 15

[[e-mail]]

Attachment: pull request redirect to github

when set MOZILLA_OFFICIAL=1, it will take default facebook app id. 

If there's any customized json exist, overwrite by the customized json.

Comment 16

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

Comment on attachment 767630 [details]
pull request redirect to github

We should re-organize application-data.js one day.

Comment 17

[[e-mail]]

merged in master-gaia be79ceb2b5d399087745a8af0da6a30d09d4d742
thanks!

Comment 18

[Jose Manuel Cantera]

I've put a relevant comment on GH that you have not paid attention. Please check

Comment 19

[[e-mail]]

Hi Jose, 

tim has replied on GH. Add default id to communications_services.json does not fit the criteria of comment 13.

Comment 20

[Jason Smith [:jsmith]]

John - This a breaking a smoke test and blocking QA testing. Can you get this uplifted asap?

Comment 21

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

[v1-train 4d6cbe4]

There are merge conflicts on v1.0.1

Comment 22

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

v1.1.0hd: 4d6cbe49fdba32f72ec8bc6280e4782c250ab41e

Comment 23

[[e-mail]]

john, please uplift bug 883510 before apply this patch to v1.0.1

Comment 24

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

Fred, the commit from 883510 does not apply cleanly to v1-train or v1.0.1.  I've flaged you and the bug assignee, but I just wanted to give you a heads up here as well that you're likely going to need to rework this patch as well.

Comment 25

[Nick Davidson]

Verified - issue no longer reproduce on Leo Build ID: 20130627070213
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/6a9ca50a313b
Gaia: fa3d64fad6f3b69be17ebbfdf4b406108afc99f4
Platform Version: 18.1
RIL Version: 01.01.00.019.138

Comment 26

[[e-mail]]

replied in bug 883510 comment 24 ,
after experiment, I think bug 883344 looks fine if bug 883510 is uplifted.

Comment 27

[Armen [:armenzg]]

I've filed a bug for the Keon Geeksphone - bug 891045

Is it fine if I ask Geeksphone to use MOZILLA_OFFICIAL=1?

Does any of you know how to talk to them? I've used their forums but they have not responded yet.