maui icon indicating copy to clipboard operation
maui copied to clipboard

VisualState setter not working for Button BackgroundColor with AOT enabled

Open Kaiffa opened this issue 2 years ago • 12 comments

Description

The setter is not working properly when in release mode. In debug it works fine. Only noticed it for the backgroundcolor on button. TextColor works fine for button. Didn't test anything other. Also only tested on android, don't now if it's the same for other platforms.

I created a simple project to demonstrate. It's basicly the templete that you get when createing a new project with added checkbox to enable/disable the button:

<CheckBox x:Name="checkBox" />
            
<Button
    x:Name="CounterBtn"
    Text="Click me"
    HorizontalOptions="Center"
    IsEnabled="{Binding Source={x:Reference checkBox}, Path=IsChecked}"/>
Before Debug Release
Screenshot_20221125-180110 Screenshot_20221125-181250 Screenshot_20221125-180059

Only the text color changed but the background didn't in release mode.

Steps to Reproduce

  1. Download linked project
  2. Set to release mode
  3. Run on local android device
  4. Click the checkbox to enable/disable the button
  5. See results

Link to public reproduction project repository

https://github.com/Kaiffa/maui-bug-visualstate-button-release

Version with bug

7.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

Android 12.0

Did you find any workaround?

No

Relevant log output

Visual studio 17.4.1

Android device used:
Samsung Galaxy A13, Android 12

Kaiffa avatar Nov 25 '22 17:11 Kaiffa

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost avatar Nov 28 '22 15:11 ghost

I tried a few things, but found something that makes me feel this is an Android AOT issue... somehow.

If I force XamlC in debug, no issue:

<_MauiForceXamlCForDebug>True</_MauiForceXamlCForDebug>

If I disable the AOT but keep trimming on, no issue:

<PublishTrimmed>true</PublishTrimmed>
<RunAOTCompilation>false</RunAOTCompilation>

However, If I now enable AOT, the issue returns:

<PublishTrimmed>true</PublishTrimmed>
<RunAOTCompilation>true</RunAOTCompilation>

@jonathanpeppers @rolfbjarne how would one go about seeing where/why this happens?

mattleibow avatar Dec 08 '22 13:12 mattleibow

There is a workaround in #8961 that seems to work as well:

Use the Background property instead of BackgroundColor and the button color changes as expected. Just need to make sure to use Brush instead of Color as the values.

mattleibow avatar Dec 08 '22 14:12 mattleibow

Comments in #8961 also seem to indicate this happens for iOS on TestFlight - probably mono AOT at this point...

mattleibow avatar Dec 08 '22 14:12 mattleibow

Related to #9753 ?

BioTurboNick avatar Jan 12 '23 04:01 BioTurboNick

/cc @fanyang-mono

steveisok avatar Jan 23 '23 16:01 steveisok

So @steveisok and @fanyang-mono - are there any working theories on why this is happening? Is there anything we can be doing on our side to move this forward?

hartez avatar Feb 01 '23 23:02 hartez

@hartez Nothing off the top of my head. I expect it's going to take a bit of sleuthing. @fanyang-mono when do you expect to be able to work on this?

steveisok avatar Feb 02 '23 16:02 steveisok

I could start working on this next week. First, I will be working on creating an app hopefully a desktop app, which could reproduce this issue. That would be easier for me to debug Mono with.

fanyang-mono avatar Feb 02 '23 17:02 fanyang-mono

Hi All,

works for me:

<Style TargetType="Button">
        <Setter Property="Background" Value="{StaticResource PrimaryButton}"/>
        <Setter Property="BackgroundColor" Value="{StaticResource PrimaryButton}"/>
        <Setter Property="TextColor" Value="{StaticResource PrimaryButtonText}"/>
        <Setter Property="CornerRadius" Value="4"/>
        <Setter Property="HorizontalOptions" Value="FillAndExpand"/>
        <Setter Property="FontSize" Value="20"/>
        <Setter Property="Padding" Value="20"/>
        <Setter Property="MinimumHeightRequest" Value="44"/>
        <Setter Property="MinimumWidthRequest" Value="44"/>
        <Setter Property="VisualStateManager.VisualStateGroups">
            <VisualStateGroupList>
                <VisualStateGroup x:Name="CommonStates">
                    <VisualState x:Name="Normal">
                        <VisualState.Setters>
                            <Setter Property="Background" Value="{StaticResource PrimaryButton}"/>
                            <Setter Property="BackgroundColor" Value="{StaticResource PrimaryButton}"/>
                            <Setter Property="TextColor" Value="{StaticResource PrimaryButtonText}"/>
                        </VisualState.Setters>
                    </VisualState>
                    <VisualState x:Name="Disabled">
                        <VisualState.Setters>
                            <Setter Property="Background" Value="{StaticResource PrimaryButtonDisabled}"/>
                            <Setter Property="BackgroundColor" Value="{StaticResource PrimaryButtonDisabled}"/>
                            <Setter Property="TextColor" Value="{StaticResource PrimaryButtonTextDisabled}"/>
                        </VisualState.Setters>
                    </VisualState>
                </VisualStateGroup>
            </VisualStateGroupList>
        </Setter>
    </Style>

denhaandrei avatar Feb 20 '23 11:02 denhaandrei

Sorry for the delay. I tried it and this is reproducible locally. I also tried using logging adb shell setprop debug.mono.log default,assembly,mono_log_level=debug,mono_log_mask=all, which hasn't provided much outstanding information regarding to the cause of this bug.

I am working with @jonathanpeppers and @mattleibow on getting more insights into the problem. Will update once we have more findings.

fanyang-mono avatar Mar 09 '23 19:03 fanyang-mono

After a vast amount of logging in places, I found that AOT affected the order of types being referenced - and thus our static fields were being initialized in "incorrect" orders.

This happened in our "Remap for Controls" section and in most cases we were replacing fields instead of adding properties. We had the correct order in Debug because we were maybe lucky, but the AOT compiler rather went by assembly at a time instead of JIT field init.

In any case, the contents of the mappers should not be affected by the order of the references. I can actually replicate the same issue if I add this code before anything in the MauiProgram:

ElementHandler.ElementMapper.GetKeys();
ViewHandler.ViewMapper.GetKeys();
ButtonHandler.Mapper.GetKeys();
Button.ControlsButtonMapper.GetKeys();
Element.ControlsElementMapper.GetKeys();
VisualElement.ControlsVisualElementMapper.GetKeys();

The issue is pretty simple now that I know what it is :)

The mappers are set up in Core like this:

  1. ElementHandler.ElementMapper adds the first level of properties
  2. ViewHandler.ViewMapper takes a reference to the ElementHandler.ElementMapper field and wraps it
  3. ButtonHandler.Mapper similarly wraps ViewHandler.ViewMapper
  4. Button.ControlsButtonMapper then comes in and this is where things go wrong: it wraps the ButtonHandler.Mapper as it is designed
  5. Element.ControlsElementMapper now comes in an wraps ElementHandler.ElementMapper and adds come more members
  6. VisualElement.ControlsVisualElementMapper then wraps and appends to Element.ControlsElementMapper

This seems like it may work, but what is actually happening in 4-6 is that the field is replaced instead of being appended. This results in step 5 (Element.ControlsElementMapper) adding fields that never make it to the Button.ControlsButtonMapper because that init code has already run and copied the old mapper from 3.

Going to look at the best way to fix this to avoid order-based-init.

mattleibow avatar Mar 10 '23 20:03 mattleibow

Not sure if related but Visual state does not seem to work in iOS Release mode either - We have a button that changes color (enabled -disabled ) depending if an entry has a value and its fine in android but in iOS in release mode only color does not change.

gabsamples6 avatar Mar 15 '23 08:03 gabsamples6

Can this be backported when the PR is complete to .Net 7 please

We are also facing this in iOS for all buttons in our app in release mode where they use Visual state

duindain avatar Mar 15 '23 22:03 duindain

@mattleibow Setting the background fixes the issue. I was stuck at this for the past one week. The background color for my button does not change in testflight builds but it worked in simulator. Thank you very much.

rthanga1 avatar Apr 14 '23 18:04 rthanga1

Until the fix is published, is there a workaround that does not involve modifying MAUI?

Dokug avatar Apr 22 '23 17:04 Dokug

I think the workaround is to use Background instead of BackgroundColor. The issue is that BackgroundColor is the old way of changing colors, but when brushes were introduced a few years ago, we just kept it. The issue is that AOT loads the assemblies in a different order and ends up not firing the correct event. So the fix is to use the Background which bypasses the old to new mapping.

mattleibow avatar Apr 23 '23 07:04 mattleibow

Is there any plan to fix this in .NET7. Using Background instead of BackgroundColor is not the solution for me as I am migrating an app where Button BackgroundColor changing from ViewModel/VisualState/Trigger at so many places so applying this every where doesn't seems to be solution to me.

Is there any Handler/Renderer way so that I can write one place and fixes everywhere.

SarthakB26 avatar May 25 '23 16:05 SarthakB26

I have migrated the Xamarin forms app to .NET MAUI. Is there any plan to fix this in .NET7. Using Background instead of BackgroundColor is not the solution for me where BackgroundColor changing from ViewModel/VisualState/Trigger at so many places so applying this every where doesn't seems to be solution to me.

Is there any Handler/Renderer way so that I can write one place and fixes everywhere. @mattleibow @jonathanpeppers @fanyang-mono

satya-prismhr avatar May 26 '23 06:05 satya-prismhr

I am afk now, so apologies for the short reply...

You can try adding a new mapper in mauiprogram.cs

ViewHandler.ViewMapper.AppendToMapping(nameof(VisualElement.BackgroundColor), (v, h) => h.UpdateValue(nameof(IView.Backgeound)));

mattleibow avatar May 26 '23 10:05 mattleibow

I am afk now, so apologies for the short reply...

You can try adding a new mapper in mauiprogram.cs

ViewHandler.ViewMapper.AppendToMapping(nameof(VisualElement.BackgroundColor), (v, h) => h.UpdateValue(nameof(IView.Backgeound)));

Hi @mattleibow, Thanks for the reply. Added the code you suggested in mauiprogram.cs, It didn't fixed the Issue. image

satya-prismhr avatar May 26 '23 11:05 satya-prismhr

Interested as well in eventual workarounds for .NET 7. My customer don't have access to the source codes, as this is in NuGet packages that they are importing.

curia-damiano avatar Jun 02 '23 09:06 curia-damiano

I am here just to tell you that the BackgroundColor doesn't change even when in debug, tested on a fresh project and building it for windows. How come that nobody tested the "isEnabled" flag on a button?

Fax avatar Jun 07 '23 21:06 Fax

As denhaandrei mentioned, the only change I needed to do in my MAUI application to avoid this problem is in the file Resources/Styles/Styles.xaml in the section <Style TargetType="Button"> it to replace BackgroundColor with Background:

--- a/mauiproject/Resources/Styles/Styles.xaml
+++ b/mauiproject/Resources/Styles/Styles.xaml
@@ -25,7 +25,7 @@

     <Style TargetType="Button">
         <Setter Property="TextColor" Value="{AppThemeBinding Light={StaticResource White}, Dark={StaticResource Primary}}" />
-        <Setter Property="BackgroundColor" Value="{AppThemeBinding Light={StaticResource Primary}, Dark={StaticResource White}}" />
+        <Setter Property="Background" Value="{AppThemeBinding Light={StaticResource Primary}, Dark={StaticResource White}}" />
         <Setter Property="FontFamily" Value="OpenSansRegular"/>
         <Setter Property="FontSize" Value="14"/>
         <Setter Property="CornerRadius" Value="8"/>
@@ -39,7 +39,7 @@
                     <VisualState x:Name="Disabled">
                         <VisualState.Setters>
                             <Setter Property="TextColor" Value="{AppThemeBinding Light={StaticResource Gray950}, Dark={StaticResource Gray200}}" />
-                            <Setter Property="BackgroundColor" Value="{AppThemeBinding Light={StaticResource Gray200}, Dark={StaticResource Gray600}}" />
+                            <Setter Property="Background" Value="{AppThemeBinding Light={StaticResource Gray200}, Dark={StaticResource Gray600}}" />
                         </VisualState.Setters>
                     </VisualState>
                 </VisualStateGroup>

phispi avatar Jul 12 '23 15:07 phispi

As denhaandrei mentioned, the only change I needed to do in my MAUI application to avoid this problem is in the file Resources/Styles/Styles.xaml in the section <Style TargetType="Button"> it to replace BackgroundColor with Background:

--- a/mauiproject/Resources/Styles/Styles.xaml
+++ b/mauiproject/Resources/Styles/Styles.xaml
@@ -25,7 +25,7 @@

     <Style TargetType="Button">
         <Setter Property="TextColor" Value="{AppThemeBinding Light={StaticResource White}, Dark={StaticResource Primary}}" />
-        <Setter Property="BackgroundColor" Value="{AppThemeBinding Light={StaticResource Primary}, Dark={StaticResource White}}" />
+        <Setter Property="Background" Value="{AppThemeBinding Light={StaticResource Primary}, Dark={StaticResource White}}" />
         <Setter Property="FontFamily" Value="OpenSansRegular"/>
         <Setter Property="FontSize" Value="14"/>
         <Setter Property="CornerRadius" Value="8"/>
@@ -39,7 +39,7 @@
                     <VisualState x:Name="Disabled">
                         <VisualState.Setters>
                             <Setter Property="TextColor" Value="{AppThemeBinding Light={StaticResource Gray950}, Dark={StaticResource Gray200}}" />
-                            <Setter Property="BackgroundColor" Value="{AppThemeBinding Light={StaticResource Gray200}, Dark={StaticResource Gray600}}" />
+                            <Setter Property="Background" Value="{AppThemeBinding Light={StaticResource Gray200}, Dark={StaticResource Gray600}}" />
                         </VisualState.Setters>
                     </VisualState>
                 </VisualStateGroup>

Yes we know of this solution but this would override all the BackgroundColor property set anywhere in the app. Which can be issue when migrating Xamarin apps where people were not using Background property at all. In that case you would have to replace BackgroundColor with Background through out the app which is not cool at all.

SarthakB26 avatar Jul 13 '23 06:07 SarthakB26

As denhaandrei mentioned, the only change I needed to do in my MAUI application to avoid this problem is in the file Resources/Styles/Styles.xaml in the section <Style TargetType="Button"> it to replace BackgroundColor with Background:

--- a/mauiproject/Resources/Styles/Styles.xaml
+++ b/mauiproject/Resources/Styles/Styles.xaml
@@ -25,7 +25,7 @@

     <Style TargetType="Button">
         <Setter Property="TextColor" Value="{AppThemeBinding Light={StaticResource White}, Dark={StaticResource Primary}}" />
-        <Setter Property="BackgroundColor" Value="{AppThemeBinding Light={StaticResource Primary}, Dark={StaticResource White}}" />
+        <Setter Property="Background" Value="{AppThemeBinding Light={StaticResource Primary}, Dark={StaticResource White}}" />
         <Setter Property="FontFamily" Value="OpenSansRegular"/>
         <Setter Property="FontSize" Value="14"/>
         <Setter Property="CornerRadius" Value="8"/>
@@ -39,7 +39,7 @@
                     <VisualState x:Name="Disabled">
                         <VisualState.Setters>
                             <Setter Property="TextColor" Value="{AppThemeBinding Light={StaticResource Gray950}, Dark={StaticResource Gray200}}" />
-                            <Setter Property="BackgroundColor" Value="{AppThemeBinding Light={StaticResource Gray200}, Dark={StaticResource Gray600}}" />
+                            <Setter Property="Background" Value="{AppThemeBinding Light={StaticResource Gray200}, Dark={StaticResource Gray600}}" />
                         </VisualState.Setters>
                     </VisualState>
                 </VisualStateGroup>

Yes we know of this solution but this would override all the BackgroundColor property set anywhere in the app. Which can be issue when migrating Xamarin apps where people were not using Background property at all. In that case you would have to replace BackgroundColor with Background through out the app which is not cool at all.

I had to solve it for my customer too, so I disabled AOT compilation in project by adding this line into my CSPROJ file in the first "PropertyGroup". <RunAOTCompilation>false</RunAOTCompilation>

This was the fastest way to solve it, since the issue is opened for more then half year...

bofden avatar Jul 13 '23 13:07 bofden

Yes we know of this solution but this would override all the BackgroundColor property set anywhere in the app. Which can be issue when migrating Xamarin apps where people were not using Background property at all. In that case you would have to replace BackgroundColor with Background through out the app which is not cool at all.

Thanks for the clarification. But does anything speak against using this workaround for apps that are freshly created with Maui?

phispi avatar Jul 13 '23 17:07 phispi

Yes we know of this solution but this would override all the BackgroundColor property set anywhere in the app. Which can be issue when migrating Xamarin apps where people were not using Background property at all. In that case you would have to replace BackgroundColor with Background through out the app which is not cool at all.

Thanks for the clarification. But does anything speak against using this workaround for apps that are freshly created with Maui?

The only thing you need to make sure if you are using this workaround is that from now wards you need to use Backgroud property to change/set backgroundcolor property of Button. Setting BackgroundColor will have no effect and will be override by the Background value set in ButtonStyle in App.xaml if not explicitly set on particular Button.

Yup For new apps this works good enough

SarthakB26 avatar Jul 14 '23 04:07 SarthakB26

I found a workaround for now that is quite simple:

Remapping the property from BackgroundColor to Background fixed the issue. This is easy to do in the maui program app builder:

.ConfigureMauiHandlers(_ => 
{
	LabelHandler.Mapper.AppendToMapping(
		nameof(View.BackgroundColor),
		(handler, View) => handler.UpdateValue(nameof(IView.Background)));
	ButtonHandler.Mapper.AppendToMapping(
		nameof(View.BackgroundColor),
		(handler, View) => handler.UpdateValue(nameof(IView.Background)));
})

mattleibow avatar Jul 21 '23 20:07 mattleibow

Will this be fixed soon, it is very fundamental for us to be able to release that the buttons show no colour on pressing. Testing in debug is of course all ok. I am beginning to despair that it is impossible to release a large Maui project to customers.

StephenWin avatar Aug 13 '23 09:08 StephenWin