Custom-URL-scheme icon indicating copy to clipboard operation
Custom-URL-scheme copied to clipboard

Cordova-Android 7.0.0 support

Open NeoLSN opened this issue 7 years ago • 17 comments

https://cordova.apache.org/announcements/2017/12/04/cordova-android-7.0.0.html

<!-- An existing config.xml -->
<edit-config file="AndroidManifest.xml" target="/manifest/application" mode="merge">

<!-- needs to change to -->
<edit-config file="app/src/main/AndroidManifest.xml" target="/manifest/application" mode="merge">

It means

 <config-file target="AndroidManifest.xml" parent="/manifest/application/activity">

Need change to

 <config-file target="app/src/main/AndroidManifest.xml" parent="/manifest/application/activity">

Could this plugin release a new version for this change?

NeoLSN avatar Dec 07 '17 06:12 NeoLSN

I love it when Cordova changes break all of my plugins :(((

Would you mind doing a PR? Cheers!

EddyVerbruggen avatar Dec 07 '17 08:12 EddyVerbruggen

If I may, I would suggest to have a look, as I do, at https://github.com/jeduan/cordova-plugin-facebook4/pull/601 to see how the same issue will be solved in cordova-plugin-facebook4

In this related PR there are some discussions about how to handle in a good way this upgrade

peterpeterparker avatar Dec 15 '17 12:12 peterpeterparker

+1

jacobweber avatar Dec 27 '17 19:12 jacobweber

Actually I don't think this improvement is needed. Just tried with a blank project. I have added [email protected] and then the current Custom-URL-Scheme plugin. Once both added, I could find the following in platforms/android/app/src/main/AndroidManifest.xml

 <intent-filter>
       <action android:name="android.intent.action.VIEW" />
        <category android:name="android.intent.category.DEFAULT" />
        <category android:name="android.intent.category.BROWSABLE" />
        <data android:scheme="mycoolapp" />
 </intent-filter>

Therefore I think no improvements are needed. Is that correct?

peterpeterparker avatar Jan 14 '18 12:01 peterpeterparker

Well, it's weird. There's some bad interaction going on when you use edit-config in your config.xml along with this plugin. Try running cordova prepare on the following config.xml (using cordova-cli 8.0.0):

<?xml version='1.0' encoding='utf-8'?>
<widget id="com.test.cordovatest" version="1.0.0" xmlns="http://www.w3.org/ns/widgets" xmlns:cdv="http://cordova.apache.org/ns/1.0">
    <name>CordovaTest</name>
    <content src="index.html" />
    <plugin name="cordova-plugin-customurlscheme" spec="4.3.0">
        <variable name="URL_SCHEME" value="x-com-sample-cordovatest" />
    </plugin>
    <edit-config file="*-Info.plist" mode="merge" target="NSCameraUsageDescription">
        <string>foo</string>
    </edit-config>
    <engine name="android" spec="7.0.0" />
    <engine name="ios" spec="4.5.4" />
</widget>

You get an error at the end:

(node:71281) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: doc.find is not a function
(node:71281) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

If you remove cordova-plugin-customurlscheme, you don't get the error.

jacobweber avatar Jan 14 '18 20:01 jacobweber

@jacobweber would you like to try the following:

<plugin name="cordova-plugin-customurlscheme" spec="^4.3.0">
    <variable name="URL_SCHEME" value="x-com-sample-cordovatest" />
    <variable name="ANDROID_SCHEME" value=" " />
    <variable name="ANDROID_HOST" value=" " />
    <variable name="ANDROID_PATHPREFIX" value="/" />
</plugin>

I've got both cordova-plugin-customurlscheme, cordova-android@7 and edit-configs in my config.xml (the NSCameraUsageDescription too) and it worked fine

peterpeterparker avatar Jan 14 '18 20:01 peterpeterparker

Hmm, I get the same result with that. Here's my project (with res, node_modules, platforms, and plugins removed). I'm doing this on a Mac with NodeJS 8.4.0.

I created it using:

cordova create CordovaTest com.test.cordovatest CordovaTest
cd CordovaTest
cordova platform add [email protected]
cordova platform add [email protected]
cordova plugin add [email protected] --variable URL_SCHEME=x-com-sample-cordovatest

then I manually added to config.xml:

    <edit-config file="*-Info.plist" mode="merge" target="NSCameraUsageDescription">
        <string>foo</string>
    </edit-config>

and ran cordova prepare. I got the same error. CordovaTest.zip

jacobweber avatar Jan 14 '18 20:01 jacobweber

@jacobweber I didn't faced any problem building your project, no error.

After unzip in the project I did

npm install
cordova plugin add cordova-plugin-customurlscheme --variable URL_SCHEME=x-com-sample-cordovatest
cordova platform add android
cordova build android

what I could think of is

  1. What's your cordova version? Did you have cordova 8 installed?
  2. I used Node v9.3.0

peterpeterparker avatar Jan 14 '18 20:01 peterpeterparker

You shouldn't need to add the platform/plugin again, since they're already in the config.xml. Try just unzipping it and running cordova prepare, or cordova prepare ios; that will restore the node modules and platforms/plugins.

I'm using cordova-cli 8.0.0. Just tried it with Node 9.4.0 and got the same result, but with more detail:

(node:75203) UnhandledPromiseRejectionWarning: TypeError: doc.find is not a function
    at Object.resolveParent (/path/to/.nvm/versions/node/v9.4.0/lib/node_modules/cordova/node_modules/cordova-common/src/util/xml-helpers.js:207:26)
    at /path/to/.nvm/versions/node/v9.4.0/lib/node_modules/cordova/node_modules/cordova-common/src/ConfigChanges/ConfigChanges.js:345:53
    at Array.forEach (<anonymous>)
    at is_conflicting (/path/to/.nvm/versions/node/v9.4.0/lib/node_modules/cordova/node_modules/cordova-common/src/ConfigChanges/ConfigChanges.js:337:17)
    at PlatformMunger.add_config_changes (/path/to/.nvm/versions/node/v9.4.0/lib/node_modules/cordova/node_modules/cordova-common/src/ConfigChanges/ConfigChanges.js:188:33)
    at /path/to/.nvm/versions/node/v9.4.0/lib/node_modules/cordova/node_modules/cordova-lib/src/cordova/prepare.js:130:32
    at _fulfilled (/path/to/CordovaTest/platforms/ios/cordova/node_modules/q/q.js:854:54)
    at self.promiseDispatch.done (/path/to/CordovaTest/platforms/ios/cordova/node_modules/q/q.js:883:30)
    at Promise.promise.promiseDispatch (/path/to/CordovaTest/platforms/ios/cordova/node_modules/q/q.js:816:13)
    at /path/to/CordovaTest/platforms/ios/cordova/node_modules/q/q.js:624:44
(node:75203) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:75203) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

jacobweber avatar Jan 14 '18 21:01 jacobweber

@jacobweber oki doki, I did and could confirm that I faced the same error as the last one you displayed

don't know why I don't face it with my main project. maybe the use another plugin, like camera, fix it. no idea

peterpeterparker avatar Jan 14 '18 21:01 peterpeterparker

@EddyVerbruggen Any chance we can get a fix for this? You can reproduce it like this, using cordova-cli 8.0.0 and Node 8.9.4:

cordova create CordovaTest com.sample.cordovatest CordovaTest
cordova create CordovaTest com.jacobweber.cordovatest CordovaTest
cordova platform add [email protected]
cordova plugin add cordova-plugin-customurlscheme --variable URL_SCHEME=x-com-test
rm -rf platforms/ node_modules/ plugins/ package-lock.json

then edit config.xml and add:

    <edit-config file="*-Info.plist" mode="merge" target="NSCameraUsageDescription">
        <string>foo</string>
    </edit-config>

then run cordova prepare --verbose, and you'll get:

(node:9484) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: doc.find is not a function
(node:9484) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

jacobweber avatar Mar 06 '18 05:03 jacobweber

@jacobweber Happy to merge a PR.

EddyVerbruggen avatar Mar 06 '18 07:03 EddyVerbruggen

After some research, I'm thinking this may be a cordova-cli bug when you have edit-config targeting Info.plist in both a plugin and config.xml, possibly related to https://issues.apache.org/jira/browse/CB-13564.

jacobweber avatar Mar 06 '18 18:03 jacobweber

any update?

gbarrionuevo avatar Mar 29 '18 15:03 gbarrionuevo

Can we get a fork @jacobweber ?

I have other plugins that are requiring me to use cordova 8, and this one that is requiring that I stay at 7. Isn't phonegap development fun 🎉 😡

codyrushing avatar Oct 16 '18 19:10 codyrushing

@codyrushing I haven't tried to fix the underlying Cordova bug myself, and Cordova development these days seems....under-resourced.

But I managed to work around it by avoiding <edit-config> in my project's config.xml. Instead I'm using <custom-config-file> from the cordova-custom-config plugin. Not ideal, but it works.

jacobweber avatar Oct 16 '18 19:10 jacobweber

@jacobweber hmm. Even if I wanted to use cordova-custom-config as a workaround, it says that it won't work with Phonegap Build, which unfortunately is a requirement for my project

codyrushing avatar Oct 16 '18 19:10 codyrushing