apache / cordova-android

Apache Cordova Android
https://cordova.apache.org/
Apache License 2.0
3.65k stars 1.54k forks source link

colors.xml resource causes platform error "Cannot set properties of null (setting 'text')" #1728

Open geoidesic opened 1 month ago

geoidesic commented 1 month ago

Bug Report

Problem

I add this into my cordova project root config.xml file in the android platform node

    <platform name="android">
        <resource-file src="res/values/colors.xml" target="app/src/main/res/values/colors.xml" />
    </platform>

The src file path does exist and it is valid XML `res/values/colors.xml

<?xml version="1.0" encoding="utf-8"?>
<resources>
  <color name="background">#F1511A</color>
</resources>

What is expected to happen?

It should copy the resource to the target without complaint.

What does actually happen?

It does copy the resource file but it also spews out a non-specific error:

Cannot set properties of null (setting 'text') 

Information

From verbose logs

Wrote out android application name "MyApp" to /Users/me/code/App-CLMP/app/App/cordova/build/platforms/android/app/src/main/res/values/strings.xml
Cannot set properties of null (setting 'text')
TypeError: Cannot set properties of null (setting 'text')
    at updateProjectSplashScreenBackgroundColor (/Users/me/code/App-CLMP/app/App/cordova/build/node_modules/cordova-android/lib/prepare.js:499:68)
    at updateProjectSplashScreen (/Users/me/code/App-CLMP/app/App/cordova/build/node_modules/cordova-android/lib/prepare.js:393:5)
    at updateProjectAccordingTo (/Users/me/code/App-CLMP/app/App/cordova/build/node_modules/cordova-android/lib/prepare.js:274:5)
    at /Users/me/code/App-CLMP/app/App/cordova/build/node_modules/cordova-android/lib/prepare.js:69:21
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Promise.all (index 0)
me@MacBook-Pro build %      

I should note that it's the error's context being after trying to write strings.xml prompted me to look for that file, and it's not being created. So /Users/me/code/App-CLMP/app/App/cordova/build/platforms/android/app/src/main/res/values/strings.xml does not exist after the platform add command is run.

Command or Code

cordova platform remove android; cordova platform add android;

Environment, Platform, Device

I'm running cordova on Mac OS X and trying to build it for Android API v13 for emulator.

Version information

Cordova: 12.0.0 (cordova-lib@12.0.1) cordova-android@13.0.0 Mac OS X: 14.5 (23F79) Android Studio Koala | 2024.1.1 Patch 1 Node: v20.15.1

Checklist

breautek commented 1 month ago

colors.xml is used by cordova framework.

By providing your own resource file you're overwriting what cordova has prepared, which is missing values that cordova expects leading to your error.

It will probably be best to choose a different filename.

geoidesic commented 1 month ago

Ok tx for the reply. I'm not sure it makes sense...

I wanted to set my own color resource for use as a background for Adaptive Icons for Android. I've since used a Drawable Vector XML instead but it would good understand why the docs specifically specify using that filename if that's not correct: https://cordova.apache.org/docs/en/12.x/config_ref/images.html#adaptive-icon-with-colors

  1. Are the docs wrong?
  2. If so, what would be correct?
  3. Also why is it failing when writing Strings.xml if it's a problem with the colors.xml filename?
breautek commented 1 month ago

Ok tx for the reply. I'm not sure it makes sense...

I wanted to set my own color resource for use as a background for Adaptive Icons for Android. I've since used a Drawable Vector XML instead but it would good understand why the docs specifically specify using that filename if that's not correct: https://cordova.apache.org/docs/en/12.x/config_ref/images.html#adaptive-icon-with-colors

  1. Are the docs wrong?
  2. If so, what would be correct?
  3. Also why is it failing when writing Strings.xml if it's a problem with the colors.xml filename?

Are the docs wrong?

Docs example is incorrect, in the sense that it will produce the issue if followed. I think the intent is to show a simple example that can be derived from. With that being said, I'm considering proposing a change to rename colors.xml to something with a cordova prefix, e.g. cdv-colors.xml. I'm a firm believer that these things should be prefixed to avoid namespace clashing and trust me, the unprefixed colors.xml has caused a lot of issues since it's also commonly used by other plugins as well. Just not sure how much of this would be a breaking change and I think it's something I need to request a buy-in to actually have it change for the next major release.

  1. If so, what would be correct?

The filenames doesn't really matter as long as it doesn't already exists (cause if it exists, you will be overwriting them and will likely break something that depends on what was previously written in them). I'd suggest prefixing to avoid namespace clashing with cordova, or any other plugin. Something that will very likely be unique to your own project.

Note that if you're creating a resource with a duplicate name (like if something already defines the background color resource, similar issues will occur. So breaking away from the examples that cordova docs shows... I'd also suggest choosing a more unique name and refrain from using "general" resource names like "background" to avoid namespace clashing with other things that might use the same resource name. Or if you do need to modify the existing resource, then edit-config can be used. Note that it's a plugin.xml directive, but it's one of the few directives that also valid to be used in config.xml).

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <color name="myAppIconBackground">#FF0000</color>
</resources>
  1. Also why is it failing when writing Strings.xml if it's a problem with the colors.xml filename?

Your log indicates that it wrote the strings.xml file successfully. That is the fs.writeFileSync call did not fail. It's unrelated to the stacktrace shown which is failing when attempting to find a specific node from colors.xml when prepping the android splashscreen stuff (e.g. it's trying to sync the preference value to the colors.xml cdv_splashscreen_background resource).

So /Users/me/code/App-CLMP/app/App/cordova/build/platforms/android/app/src/main/res/values/strings.xml does not exist after the platform add command is run.

The file creation happens on the prepare step, which I don't think is ran on platform add (though I don't remember this clearly so I could be wrong). The cordova prepare command is implicitly ran on the cordova build command.

If prepare is indeed running on platform add, but that file is still not being produced while the CLI is erroring on preparing the colors.xml/splashscreen (or any other non-zero exit code) I wouldn't fret too much because it could be nuances with OS signaling that the file is written but it's not actually flushed to disk and the error is causing the process to exit too early without waiting for flushes. In normal operations, the node program will not fully exit until all data streams are flushed, but that might not be the case if the node program is exiting early due to an error.

github-throwaway-1111 commented 1 month ago

Docs example is incorrect, in the sense that it will produce the issue if followed. I think the intent [of the docs] is to show a simple example that can be derived from.

The docs flat-out say to create a file named colors.xml. It's stated it in such a way that leaves little to no room for interpretation.

misleading_docs

To think that following the instructions from the official docs breaks your Cordova project in a way that's difficult to diagnose is less than ideal.