Skip to content
This repository has been archived by the owner on Oct 24, 2022. It is now read-only.

Fix for directory structure whatever cordova-android version. #621

Closed
wants to merge 1 commit into from
Closed

Fix for directory structure whatever cordova-android version. #621

wants to merge 1 commit into from

Conversation

kevin-lot
Copy link

@kevin-lot kevin-lot commented Feb 7, 2018

Fixes #599
Update plugin.xml.
<config-file> interperts correctly path for AndroidManifest.xml, strings.xml and config.xml whatever version of directory structure.
I just remove facebookconnect.xml and move values inside strings.xml.

Tested with facebook connection under cordova-android-7.

note: Apply standardjs fixes on after_prepare.js too (sorry about that).

Update plugin.xml.
config-file interperts correctly path for AndroidManifest.xml, strings.xml and config.xml whatever version of directory structure.
Apply standardjs on after_prepare.js.
@kevin-lot kevin-lot changed the title Fix for directory whatever cordova-android version. Fix for directory structure whatever cordova-android version. Feb 7, 2018
@fredgalvao
Copy link
Collaborator

Don't take my question personally, I'm just trying to simplify things:
Considering my personal opinion, which some online sources agree with, that strings.xml should not be modified by a plugin, but only by a user or a localization process/tool, and also considering that #601 already aims at fixing the same issue successfully, why do we need this PR over that one?

@kevin-lot
Copy link
Author

Sorry, for the misunderstanding. I thought that the PR #601 did not solve the problem because it is not compatible with earlier versions of Cordova-android.
It dates from December 9 and is still waiting. So I thought you wanted extra compatibility.

@fredgalvao
Copy link
Collaborator

That's actually a very good point (extra backwards compatibility). That makes this a valid alternative to the original PR.

As for the delay there, it's just that there isn't that many active developers available to help maintain this plugin right now, so it's just fair that it takes a while for things to move. Jeduan has offered to transfer ownership of the repo to someone willing to lead maintenance, but we haven't had someone say "Pick me!" yet, which is sad.

@sebj54
Copy link

sebj54 commented Feb 23, 2018

I confirm it works! I made a custom manual diff patch of this PR and added the plugin from a local copy.
The only tweak I had to do was running app with this command:

cordova run android -- --minSdkVersion=16

Otherwise, I got this error:

:app:processDebugManifest FAILED
24 actionable tasks: 24 executed

        uses-sdk:minSdkVersion 15 cannot be smaller than version 16 declared in library [:CordovaLib] C:\Users\sebas\sites\sample-project\platforms\android\CordovaLib\build\intermediates\manifests\full\debug\AndroidManifest.xml as the library might be using APIs not available in 15
        Suggestion: use a compatible library with a minSdk of at most 15,
                or increase this project's minSdk version to at least 16,
                or use tools:overrideLibrary="org.apache.cordova" to force usage (may lead to runtime failures)

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:processDebugManifest'.
> Manifest merger failed : uses-sdk:minSdkVersion 15 cannot be smaller than version 16 declared in library [:CordovaLib] C:\Users\sebas\sites\sample-project\platforms\android\CordovaLib\build\intermediates\manifests\full\debug\AndroidManifest.xml as the library might be using APIs not available in 15
        Suggestion: use a compatible library with a minSdk of at most 15,
                or increase this project's minSdk version to at least 16,
                or use tools:overrideLibrary="org.apache.cordova" to force usage (may lead to runtime failures)

Like you said, it is really sad that there is not even one Cordova plugin for Facebook Login actually maintained :(

@fredgalvao fredgalvao mentioned this pull request Mar 22, 2018
@akhatri
Copy link

akhatri commented May 26, 2018

Hi guys - thanks for attempting to get this working for the new Android version :)

I just tried this PR version but it doesnt work on my project. I'm not sure if its something to do with my setup. Its identical to this issue on SO https://stackoverflow.com/questions/48706617/ionic-2-app-build-fail-after-install-facebook-login-plugin.

Adding these manually in the xml file works but I'm not sure if it will work this way as this folder is always re-built.

My system information

cli packages:

    @ionic/cli-utils  : 1.19.2
    ionic (Ionic CLI) : 3.20.0

global packages:

    cordova (Cordova CLI) : 8.0.0

local packages:

    @ionic/app-scripts : 3.1.9
    Cordova Platforms  : android 7.1.0
    Ionic Framework    : ionic-angular 3.9.2

System:

    Android SDK Tools : 26.1.1
    Node              : v8.11.1
    npm               : 5.6.0
    OS                : Windows 10

I've tried to play around with the paths within the config.xml file too but no luck. The new paths as per tha latest cordova android is platforms/android/app/src/main/res/values/strings.xml

@peterpeterparker
Copy link
Collaborator

@akhatri see the documentation, chapter "cordova-android >= 7"

https://github.com/jeduan/cordova-plugin-facebook4/blob/master/docs/android/README.md

@akhatri
Copy link

akhatri commented May 27, 2018

Hi @peterpeterparker it's working now :) I had to remove the Android platform and then build the project so it took the config settings. Previously the project was already built so it appears that it didn't override it.

when can we merge this PR into the master branch? Would be great to get this plugin working on our project.

@noahcooper
Copy link
Collaborator

This plugin is deprecated. Check out cordova-plugin-facebook-connect at https://www.npmjs.com/package/cordova-plugin-facebook-connect. That plugin requires cordova-android 7, so does not need to worry about the different directory structures of older Android versions anymore.

@noahcooper noahcooper closed this Feb 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cordova-android@7.0.0 incompatibility
6 participants