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): improved merge of <uses-feature/> elements #11110

Merged
merged 1 commit into from Aug 20, 2019

Conversation

jquick-axway
Copy link
Contributor

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

Summary:

  • TIMOB-27304 Fixed so that Android setting <uses-feature/> required true in "tiapp.xml" should override false setting in libraries.
    • Merge is supposed to do a logical-OR on the "android:required" value.
  • Added "tools:replace" attribute support to <uses-feature/>.
    • Allows overriding "android:required" to false if set true by library. (Only means of doing this.)

Test:

  1. Create a Classic Titanium app project.
  2. Add module "ti.barcode" to "tiapp.xml". (Download from here.)
  3. Add module "ti.nfc" to "tiapp.xml". (Download from here.)
  4. Add the <uses-feature/> settings in "tiapp.xml" as shown below.
  5. Build for Android.
  6. Open Mac's "Finder" window. (Or "Windows Explorer" on Windows.)
  7. Go to project subdirectory: ./build/android
  8. Open file "AndroidManifest.xml".
  9. Verify following XML element has no "required" attribute or it is set true.
    <uses-feature android:name="android.hardware.camera"/>
  10. Verify following XML element has "required" set to false.
    <uses-feature android:name="android.hardware.nfc" android:required="false"/>
  11. Verify following XML element has "required" set to true.
    <uses-feature android:name="android.hardware.touchscreen" android:required="true"/>

tiapp.xml

<?xml version="1.0" encoding="UTF-8"?>
<ti:app xmlns:ti="http://ti.appcelerator.org">
	<android xmlns:android="http://schemas.android.com/apk/res/android">
		<manifest>
			<uses-feature android:name="android.hardware.camera" android:required="false"/>
			<uses-feature android:name="android.hardware.nfc" android:required="false" tools:replace="android:required"/>
			<uses-feature android:name="android.hardware.touchscreen" android:required="true"/>
		</manifest>
	</android>
	<modules>
		<module platform="android">ti.barcode</module>
		<module platform="android">ti.nfc</module>
	</modules>
</ti:app>

- [TIMOB-27304] Fixed so that Android setting <uses-feature/> required "true" in "tiapp.xml" should override "false" setting in libraries.
  * Merge is supposed to do a logical-OR on the "android:required" value.
- Added "tools:replace" attribute support to <uses-feature/>.
  * Allows overriding "android:required" to "false" if set "true" by library. (Only means of doing this.)
@build
Copy link
Contributor

build commented Aug 3, 2019

Warnings
⚠️

🔍 Can't find junit reports at ./junit.*.xml, skipping generating JUnit Report.

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

Generated by 🚫 dangerJS against a2f676d

Copy link
Contributor

@ypbnv ypbnv 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 Aug 5, 2019

FR passed. Correct value of elements in manifest file based on OR(ing) of values from all the involved xml files.
Env:
Mac OS 10.14.3
Ti SDK: 8.2.0.v20190802180802
Appc CLI: 7.1.0-24
Node: 8.16.0
JDK: 10.0.2

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