Update the config.xml path in the XHR load eventListener

[ghost]

Platforms affected

I am using macOS Mojave, but I think this should affect Windows and Linux as well. This is a fix for the Electron platform in Cordova.

Motivation and Context

This makes the configuration settings in config.xml work when targeting the Electron platform when running Cordova.

I am specifically working with the Splashscreen Cordova plugin and the settings were not loading when Cordova targeting Electron, like this:

cordova run electron --livereload

The settings I tried to pull in is in the root of my project in config.xml:

<platform name="electron">
    <icon src="assets/imgs/icon.png" />
    <splash src="assets/imgs/splash.png" />
    <preference name="SplashScreen" value="assets/imgs/splash.png" />
    <preference name="SplashScreenWidth" value="2732" />
    <preference name="SplashScreenHeight" value="2732" />
</platform>

Description

I changed cordova-js-src/confighelper.js and cordova-lib/cordova.js to load config.xml from the relative location and not the root. (I just removed the "/" in front of config.xml.)

Testing

Ran all tests.

Checklist

• [Yes ] I've run the tests to see all new and existing tests pass
• [N/A] I added automated test coverage as appropriate for this change
• [N/A] Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• [N/A] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• [N/A] I've updated the documentation if necessary

[erisu]

@jackvz I have also seen this issue and been played around with the same changes you applied in this PR.

I believe these changes will be OK.

Have you tested your changes with following:

• Building and application with cordova build and running the built app.
• Running the application with cordova run

Also, this has nothing to do with Ionic specifically. This is about the normal process for loading Cordova's config.xml for the splash screen plugin, and potentially other plugins as well.

Can you update the title and description to be more accurate to the problem so that other users will understand that its more around the cordova plugin usage and not Ionic.

[ghost]

Hi @erisu. Thanks for checking. Right, I have updated the title and description. Yes, I have now tested it by building and running with Cordova, and it's working as expected.

[codecov-io]

Codecov Report

Merging #42 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #42   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          14     14           
  Lines         568    568           
=====================================
  Hits          568    568

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95ca4de...18b4516. Read the comment docs.

[ghost]

Hi @erisu and all. How does this work? This actually solves the issue for me, not only in Ionic but more importantly also in pure Cordova. Not sure if this is set up like it is currently because of some other integration, and in that case I am happy to use my fork. Please let me know if there is anything else for me to do about this pull request. Happy Friday!

[erisu]

@jackvz I will run a few tests against your PR and make sure that there was no missing use case that I wasn't thinking about.

If all is good, I will try to merge into master this coming week. As for the patch release timeline, I can't give an exact date as of yet. There are a few things I want to check before release.

[ghost]

Sounds good @erisu, thanks. No rush.

[GedasGa]

I’ve run a few tests. Config.xml loads correctly. However, the splash screen for browser and electron only works when the images are in the {project_root}/www/ folder, but if you build for other platforms, you might not want your splash screen to reside in the{project_root}/www/. We’re looking for a possible solution.

Other than that, this PR looks good to me. I just feel that the PR message could be more accurate. Something like “Correctly load the config.xml” or “Update the config.xml path in the xhr load eventListener”.

[GedasGa]

LGTM @erisu

[ghost]

@GedasGa I get that, and I'm not sure what the best solution is. I'm still getting my head around Cordova. The image need not be in the root. Here is a demo of how the splash screen image is in a resources folder in root. I did have to copy the splash image and config.xml file to make it work for both the Electron and Browser targets, but I believe we can use Cordova Hook scripts for that.

[erisu]

👍