fix: Color white statusbar

[gabriel2mm]

Platforms affected

• Android

Motivation and Context

This change fixes an issue where the color #FFFFFFFF (fully opaque white) is treated as an invalid color when used for the status bar background.

The problem happens because Color.parseColor("#FFFFFFFF") correctly returns -1, which collides with the current sentinel value used for INVALID_COLOR.

As a result, valid white colors cannot be applied to the status bar.

fixes: #1938

Description

Updated the invalid color handling to avoid collisions with valid ARGB color values returned by Color.parseColor().

This allows colors such as #FFFFFFFF to be parsed and applied correctly.

Example of an invalid sentinel collision:
window.statusbar.setBackgroundColor('#80000000');

This returns a value equal to Integer.MIN_VALUE.

Testing

• npm t
• Build testing

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• 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)
• I've updated the documentation if necessary

[GitToTheHub]

Hi good catch. Since you already noted that #80000000 resolves to Integer.MIN_VALUE the change would be to not use INVALID_COLOR at all. INVALID_COLOR in most cases is set, when a string could not parsed by Color.parseColor, which then throws an IllegalArgumentException. So the right approach would be to react on the exception directly instead of setting INVALID_COLOR and use this.

These methods set INVALID_COLOR:

• parseColorFromString will return INVALID_COLOR if the parameter string is empty or Color.parse throws IllegalArgumentException. parseColorFromString is called by:
  • setStatusBarBackgroundColor which is called from the corresponding JavaScript function
  • getPreferenceStatusBarBackgroundColor for reading the preference StatusBarBackgroundColor
• getPreferenceBackgroundColor returns INVALID_COLOR if the preference BackgroundColor could not be parsed as int

The variable overrideStatusBarBackgroundColor is an int and can also contain INVALID_COLOR. It is set by setStatusBarBackgroundColor which is called by the corresponding JavaScript function. It is only used in updateSystemBars. Instead of an int, this could contain the string given by setStatusBarBackgroundColor and parse it every time in updateSystemBars to catch the exception.

[gabriel2mm]

I agree with the points raised. I’ve pushed a new commit for review where I removed the INVALID_COLOR. I believe INVALID_COLOR no longer makes sense in this functionality and could be handled differently. @GitToTheHub could you please take another look?

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.43%. Comparing base (2450217) to head (07467c0).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1939   +/-   ##
=======================================
  Coverage   61.43%   61.43%           
=======================================
  Files          24       24           
  Lines        4922     4922           
=======================================
  Hits         3024     3024           
  Misses       1898     1898           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dpogue]

It might be safe to use 0 (0x00000000, or #00000000 which is transparent: rgba(0, 0, 0, 0)) as a sentinel value. That's what's being proposed as a "null" value for the Android WebView implementation of theme-color handling.

[gabriel2mm]

It might be safe to use 0 (0x00000000, or #00000000 which is transparent: rgba(0, 0, 0, 0)) as a sentinel value. That's what's being proposed as a "null" value for the Android WebView implementation of theme-color handling.

I had written this to you before, but I changed my mind afterward, and I’ll try to explain why:

Using 0 does not work simply because, on some Android devices, 0x00000000 represents the initial state of the status bar. See:

When attempting to change the color and then revert it back to the initial state, we get blocked due to the sentinel value being 0.

I still have the feeling that handling it the way I proposed would be a better approach.

[gabriel2mm]

If possible, I’d appreciate it if someone from the community could review this PR so we can determine whether it’s viable to merge it.
I’m committed to supporting this version of Cordova with fixes and improvements.

[breautek]

Tests passes and I think this is definitely an improvement.

We can't really reserve an int for INVALID_COLOR because any value within the int range is a valid colour, assuming you have a 4x 8bit channels (that's you're RGBA).

If wanted to reserve a number value for invalid color and not have it conflict, we'd have to pick a value outside of an int32 range, which means using Long value.

Or we can do what this PR does and use String / null which I think is perfectly reasonable as well provided there is null guarding. Looks like all APIs being modified here are private methods, so it should be relatively safe change.

Assuming there is no other objections, ping me in a couple days and I'll merge.

[GitToTheHub]

Previously overrideStatusBarBackgroundColor did get the parsed color here.
I would change overrideStatusBarBackgroundColor to Integer and set the parsed color directly here, e.g.:

overrideStatusBarBackgroundColor = parseColorFromString(String.format("#%02X%02X%02X%02X", a, r, g, b));

So it's the same behaviour as before and is null if the parsing failed

[GitToTheHub]

When you change overrideStatusBarBackgroundColor to an Integer, you can call updateStatusBar(overrideStatusBarBackgroundColor) again instead of updateSystemBars(). Or is there any other reason to call updateSystemBars() instead of updateStatusBar()?

[GitToTheHub]

When overrideStatusBarBackgroundColor is an Integer this can be removed.

[GitToTheHub]

Why did you removed the fallback getUiModeColor()?