gasolin@mozilla.com Assignee
	Description • 11 years ago

hide SIM contact import when device not support telephony

	gasolin@mozilla.com Assignee
	Comment 1 • 11 years ago

hide sim import section by reusing 'this.mobConn' to detect if window.navigator.mozMobileConnection not exist

	Borja Salguero [:borjasalguero]
	Comment 2 • 11 years ago

Taking a look to the patch! :)

	Borja Salguero [:borjasalguero]
	Comment 3 • 11 years ago

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

Small comment to address. Please add a test for the fix! Feedback+ and waiting your changes for making the final review with changes required (patch works great btw :)).
Please ask me to review when you will be ready! Thanks! Gracias ;)

	gasolin@mozilla.com Assignee
	Comment 4 • 11 years ago

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

I've add testcase for no telephony case.
Please kindly review it again, thanks

	gasolin@mozilla.com Assignee
	Comment 5 • 11 years ago

Hi, anything I can help for this issue?

	Borja Salguero [:borjasalguero]
	Comment 6 • 11 years ago

Im taking a look right now! :)

	Borja Salguero [:borjasalguero]
	Comment 7 • 11 years ago

Hi Fred,
I've sent you a small fix to the tests (it's not related with your patch, but I think is needed for checking that new tests are testing our real HTML :) ). Link here https://github.com/gasolin/gaia/pull/2 . Could you take a look? Thanks!!

	gasolin@mozilla.com Assignee
	Comment 8 • 11 years ago

looks good, merge to this PR. thanks!

	Borja Salguero [:borjasalguero]
	Comment 9 • 11 years ago

Fred, one doubt. When you mean that 'mozMobileConnection' is not available, you mean that we have a device without SIM? How can I test the patch? Thanks!

	gasolin@mozilla.com Assignee
	Comment 10 • 11 years ago

(assume you are in oslo) please go to george to test on tablet (which without SIM)

	(Inactive after June) George Duan [:gduan] [:喬智]
	Comment 11 • 11 years ago

Hi Fred,
I believe you already tried on another tablet.
However, it seems not work on the device I have, mozMobileConnection still exist in navigator.

Hi Viral,
will window.navigator.mozMobileConnection still exist on tablet device? Currently, we use it to disable some communication function in gaia. I think we should perhaps remove it if the device doesn't support it..

	viral [:viralwang]
	Comment 12 • 11 years ago

(In reply to George Duan [:gduan] from comment #11)
> will window.navigator.mozMobileConnection still exist on tablet device?
> Currently, we use it to disable some communication function in gaia. I think
> we should perhaps remove it if the device doesn't support it..

So far we still use window.navigator.mozMobileConnection in tablet but it will not work in flatfish. 
I'm thinking we should have some generic control in gaia to define what function we should support, otherwise different functions will use their own way to check the function is needed or not.

	gasolin@mozilla.com Assignee
	Comment 13 • 11 years ago

(add tim in loop)

Viral, do you mean we still HAVE `window.navigator.mozMobileConnection` API in flatfish but that API is not work? What default return value will be?


per discussion with tim, it's more reasonable to not provide such API(window.navigator.mozMobileConnection/mozTelephony) if telephony is not support. Then developer could use feature detection approach as they do in general web.

	gasolin@mozilla.com Assignee
	Comment 14 • 11 years ago

after some diving to customization code, I found we can disable dom api by add partner-prefs.js in gaia customization.

ex:

prefs.push(['dom.navigator-property.disable.mozMobileConnection', true]);

@yuren, is it right?

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)
	Comment 15 • 11 years ago

I didn't use this feature but looks it will be included into user.js if exists in distribution directory.

	gasolin@mozilla.com Assignee
	Comment 16 • 11 years ago

just test user_pref('dom.navigator-property.disable.mozMobileConnection', true/false); on real device, seems not work though. 

If we decide to use this approach, we still need gecko expose such user_pref.

	(Inactive after June) George Duan [:gduan] [:喬智]
	Comment 17 • 11 years ago

Hi Vicamo,

Per offline dicussion, please kindly open a new bug as comment 16 , we need to be able to set dom.navigator-property.disable.mozMobileConnection in user_pref.

Thanks.

	Borja Salguero [:borjasalguero]
	Comment 18 • 11 years ago

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

Im gonna remove the review because this needs some Gecko work yet. Let me know when this is ready for testing and I'll come back to review it! Thanks :)

	gasolin@mozilla.com Assignee
	Comment 19 • 11 years ago

We didn't do the real-device test but rely on an un-sync assumption.

Thanks Borja to bring it on to the table.

	viral [:viralwang]
	Comment 20 • 11 years ago

(In reply to Fred Lin [:gasolin] from comment #13)
> (add tim in loop)
> 
> Viral, do you mean we still HAVE `window.navigator.mozMobileConnection` API
> in flatfish but that API is not work? What default return value will be?

Sorry for confusing here, what I mean API is not work is that we don't need those the telephony function in flatfish. So the information like 'no sim card' is no need on our usage.

	gasolin@mozilla.com Assignee
	Comment 21 • 11 years ago

test on real device and works well

	gasolin@mozilla.com Assignee
	Comment 22 • 11 years ago

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

We've finally have a device that disabled mozConnection API, the patch works well on it

	Jason Smith [:jsmith]
	Comment 23 • 11 years ago

Flatfish has been moved to 1.4, so moving to 1.4?

	Borja Salguero [:borjasalguero]
	Comment 24 • 11 years ago

Hi Fred! Could :alive take a look to the device with this patch? Im gonna add a 'feedback?' flag, so if he confirm that the behavior is the right one in the Tablet (I have no device for testing :( ) I'll review the patch again! Thanks!

	Borja Salguero [:borjasalguero]
	Comment 25 • 11 years ago

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

Hi Alive! Could you take a look to this patch tested in the real device? I would like to ensure that it's working on the device, but I have not a real one! Could you feedback+ if it's working? Thanks!

	(inactive after 6/18) Alive Kuo [:alive]
	Comment 26 • 11 years ago

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

f+ from seeing fred's device.

	gasolin@mozilla.com Assignee
	Comment 27 • 11 years ago

I've rewrote some unit test and reuse the shared/load_body_html_helper, so FTU test not need to use mock_html_import anymore, but use the real index.html to construct DOM.

	Borja Salguero [:borjasalguero]
	Comment 28 • 11 years ago

Great job Fred! It's working perfectly on the device! R+ from my side.