Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(android): improve Ti.Geolocation API reliability in 9.0.0 #11450

Merged
merged 4 commits into from Jan 30, 2020

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Jan 25, 2020

JIRA:
https://jira.appcelerator.org/browse/TIMOB-27739

Summary:

  • As of Titanium 9.0.0, "device" or "production" builds would no longer auto-add below permissions to "AndroidManifest.xml" if JS used Ti.Geolocation APIs.
    • android.permission.ACCESS_COARSE_LOCATION
    • android.permission.ACCESS_FINE_LOCATION
  • The Ti.Geolocation APIs would never work if play-services-base library was included and play-services-location library was missing.
    • Added check for classes in both libraries before using Fused Location feature.
    • Can be an issue in 9.0.0 since app devs can set up their own custom build.gradle file.

Test:

  1. Create a Classic app project.
  2. Add the below "app.js" code to project.
  3. Add the below "build.gradle" file to project.
  4. Build and run on an Android device. (Not the emulator.)
  5. Verify that app prompts you for location permission. Tap the "Allow" button.
  6. Verify app fetches location data and shows it onscreen.

./platform/android/build.gradle

dependencies {
	implementation 'com.google.android.gms:play-services-base:17.1.0'
}

./Resources/app.js

function monitorLocation() {
	function start() {
		Ti.Geolocation.addEventListener("location", function(e) {
			label.text = JSON.stringify(e, null, 4);
			Ti.API.info("@@@ location received: " + JSON.stringify(e));
		});
	}
	var hasPermission = Ti.Geolocation.hasLocationPermissions(Ti.Geolocation.AUTHORIZATION_WHEN_IN_USE);
	if (!hasPermission) {
		Ti.Geolocation.requestLocationPermissions(Ti.Geolocation.AUTHORIZATION_WHEN_IN_USE, function(e) {
			Ti.API.info("@@@ permission result: " + JSON.stringify(e));
			if (e.success) {
				start();
			} else {
				label.text = "\nFailed to acquire location permission from end-user.";
			}
		});
	} else {
		start();
	}
}
var window = Ti.UI.createWindow();
var scrollView = Ti.UI.createScrollView({
	layout: "vertical",
	scrollType: "vertical",
	width: Ti.UI.FILL,
	height: Ti.UI.FILL,
});
var label = Ti.UI.createLabel({
	text: "\nWaiting for GPS data...",
	top: (Ti.App.iOS ? "25dp" : "5dp"),
	height: Ti.UI.SIZE,
	left: "5dp",
	right: "5dp",
	bottom: "5dp",
});
scrollView.add(label);
window.add(scrollView);
window.addEventListener("open", function(e) {
	monitorLocation();
});
window.open();

- When JS files use Ti.Geolocation APIs, build system is supposed to auto-add below permissions to AndroidManifest.xml file:
  * android.permission.ACCESS_COARSE_LOCATION
  * android.permission.ACCESS_FINE_LOCATION
- This broke when migrating to gradle. Re-adding support.
- The Ti.Geolocation APIs would never work if "play-services-base" lib was included, but "play-services-location" lib was missing.
- Now checking for classes in both libs before proceeding to use Fused Location feature.
- This can happen in Titanium 9.0.0 now that app devs can set up their own custom "build.gradle" file which specifies its own lib dependencies.
@build
Copy link
Contributor

build commented Jan 25, 2020

Messages
📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 4484 tests are passing.
(There are 456 skipped tests not included in that total)

Generated by 🚫 dangerJS against c0ad7f1

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR: PASS

@ssekhri
Copy link

ssekhri commented Jan 27, 2020

FR Passed. Prompt for location services shown and used successfully.
Verified on:
Mac OS: 10.15.1
SDK: 9.0.0.v20200127105953
Appc CLI: 7.1.2
JDK: 11.0.4
Node: 10.16.3
Studio: 6.0.0.201911251516
Device: Pixel2(v9.0) device, Nexus4(v5.1.1) device, Pixel3(v10.0) emulator,

@sgtcoolguy sgtcoolguy merged commit b8cc24a into tidev:master Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants