Maui icon indicating copy to clipboard operation
Maui copied to clipboard

[BUG] Color conversion not behaving as expected

Open reck1610 opened this issue 6 months ago • 5 comments

Is there an existing issue for this?

  • [x] I have searched the existing issues

Did you read the "Reporting a bug" section on Contributing file?

  • [x] I have read the "Reporting a bug" section on Contributing file: https://github.com/CommunityToolkit/Maui/blob/main/CONTRIBUTING.md#reporting-a-bug

Current Behavior

I had 168 failing test due to my locale.

failed CommunityToolkit.Maui.UnitTests.Converters.ColorToCmykaStringConverterTests.ColorToCmykaStringConverterValidInputTest(red: 1, green: 0, blue: 0, alpha: 1, expectedResult: "CMYKA(0%,100%,100%,0%,1)") (1ms)
  Assert.Equal() Failure: Values differ
  Expected: CMYKA(0%,100%,100%,0%,1)
  Actual:   CMYKA(0 %,100 %,100 %,0 %,1)
    at CommunityToolkit.Maui.UnitTests.Converters.ColorToCmykaStringConverterTests.ColorToCmykaStringConverterValidInputTest(Single red, Single green, Single blue, Single alpha, String expectedResult) in <redacted>\CommunityToolkit.Maui\src\CommunityToolkit.Maui.UnitTests\Converters\ColorToCmykaStringConverterTests.cs:148
    at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
    at System.Reflection.MethodBaseInvoker.InvokeWithManyArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

Not all tests for the color conversion fail. Some will generate the test data using my locale and not fail. Both my locale and the Invariant locale are not creating the correct output as described in the annotation comment and tested for in the hardcoded test data. The "en-US" locale will produce the correct output.

Expected Behavior

CommunityToolkit.Maui.Core.Extensions.ColorConversionExtensions generate the same output on all locales. CommunityToolkit.Maui.Core.Extensions.ColorConversionExtensions generate the output described in annotation comment. The Tests check for the output described in annotation comment using multiple locales.

Steps To Reproduce

  1. Set locale to non "en-US" e.g. "de-DE"
  2. Run tests

Link to public reproduction project repository

Environment

- .NET MAUI CommunityToolkit: 12.2.0
- OS: Windows 11
- .NET MAUI: -

Anything else?

No response

reck1610 avatar Sep 04 '25 08:09 reck1610

@TheCodeTraveler I dont think https://github.com/CommunityToolkit/Maui/labels/needs%20reproduction is correct.

There is no code required to reproduce this. It depends on the locale the test is run in. I changed the relevant test cases to cultured test cases to make the problem reproduce in the tests. That commit (https://github.com/CommunityToolkit/Maui/pull/2874/commits/bd18c240258498c54ff25ec9d808d2374fc738f1) could be used to reproduce the issue without changing your locale.

reck1610 avatar Sep 06 '25 09:09 reck1610

I understand that you have unit tests failing. The above description describes the scenario you're experiencing.

However, I don't understand the problem you're trying to solve.

Expected Behavior CommunityToolkit.Maui.Core.Extensions.ColorConversionExtensions generate the same output on all locales.

Should it? Is this the expected behavior? Help us understand why.

Are you asking us to add a new feature that allows for CultureInfo to be ignored by the converter? Are you saying that devs don't want to use CultureInfo and that our converters should ignore it? Is this a problem outside of your specific unit testing scenario that is affecting your production app?

I need your help to describe the issue we're trying to solve and to provide a reproduction sample of the problem that demonstrates this issue. Without both of these items, it's difficult for us to help solve the problem.

Please edit this Issue to describe the problem.

TheCodeTraveler avatar Sep 06 '25 14:09 TheCodeTraveler

I believe the issue is that when using specific cultures a comma will be used as a decimal separator, if the string generated separates the with a comma in this culture it will leave a string that cannot be parsed back to a Color.

bijington avatar Sep 06 '25 15:09 bijington

CommunityToolkit.Maui.Core.Extensions.ColorConversionExtensions should generate the same output on all locales, because localized output is different from the documented output of the methods. I have a more detailed explaination here: https://github.com/CommunityToolkit/Maui/pull/2874#issuecomment-3262084540.

To summarize the problem. The output should always look like this: CMYKA(100%,65%,52%,0%,0.625) but it can look like this:

CMYKA(100 %,65 %,52 %,0 %,0.625)
CMYKA(100%,65%,52%,0%,0.625)
CMYKA(100%,65%,52%,0%,0,625)
CMYKA(100٪؜,65٪؜,52٪؜,0٪؜,0٫625)
CMYKA(100‎%‎,65‎%‎,52‎%‎,0‎%‎,0,625)
CMYKA(100 %,65 %,52 %,0 %,0,625)
CMYKA(100 ٪,65 ٪,52 ٪,0 ٪,0٫625)
CMYKA(% 100,% 65,% 52,% 0,0,625)
CMYKA(100٪,65٪,52٪,0٪,0٫625)
CMYKA(%100,%65,%52,%0,0,625)
CMYKA(100%,65%,52%,0%,0٫625)

My changes in #2874 ensure that the output of all color conversion extensions is the same regardless of locale.

The extensions methods for RGBA, CMYKA and HSLA had an optional CultureInfo parameter that was passed as IFormatProvider for .ToString() on the alpha value. This caused unparsable output like RGBA(0,0,0,0,5). I overloaded those extension methods and marked them obsolete to inform anyone using those methods about them being broken,

reck1610 avatar Sep 06 '25 17:09 reck1610

This issue has a similar theme to the issues with the MathExpressionConverter being sensitive to CultureInfo.CurrentCulture changes when it shouldn't. The resolution was in the lines of: 1. Introducing a changeable CultureInfo Culture parameter 2. Technically only CultureInfo.InvariantCulture is supported 3. There's merit in turning on the CA1305 analyzer in the project to avoid similr bugs (see https://github.com/CommunityToolkit/Maui/pull/2722#discussion_r2149114652)

The issue can be resolved by applying the IFormatProvider overload of ToString(). In the future, enabling the CA1305 analyzer in the project will help avoid similar bugs (see https://github.com/CommunityToolkit/Maui/pull/2722#discussion_r2149114652)

stephenquan avatar Nov 17 '25 02:11 stephenquan