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

CB-14085: Fix iOS usage description #106

Closed
wants to merge 9 commits into from
Closed

CB-14085: Fix iOS usage description #106

wants to merge 9 commits into from

Conversation

slkerndnme
Copy link

@slkerndnme slkerndnme commented May 11, 2018

Platforms affected

iOS

What does this PR do?

This PR makes the plugin works properly on iOS without extra changes in the config.xml and without conflicts with other plugins. The README.md is updated accordingly.

What testing has been done on this change?

This PR replaces the tag edit-config by the tag config-file in the test context plugin.xml.

The cordova documentation imply config-file is designed and tested for iOS plist.info when edit-config is designed for more complex xml syntax, like an android manifest.xml file. The merge mode available for edit-config is designed for merging up two tags with attributes (xml tags in a plist have no attributes).

According the documentation, using edit-config on a plist.info file is not a secure practice. Adding the mode attribute to "merge" makes it worst.

If a third plugin make wrong things in its plugin.xml, it may create issues and conflicts, but it won't be coming from this PR.

This PR is tested and reliable.

@slkerndnme slkerndnme closed this May 11, 2018
@slkerndnme slkerndnme reopened this May 11, 2018
@jcesarmobile
Copy link
Member

Thanks for the PR, but we removed this on purpose because plugin variables cause conflicts with other plugins setting the same fields.
Users have to set the values using the edit-config tag as explained on the README

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants