Xamarin.forms: Crash XF when Font Style Applied to StackLayout/Grid[Bug]

[ianvink]

I mistakenly applied a Style to a StackLayout which has font information. XF 3.x and 4.x crash in the XF Xamarin.Forms.FontElement.FontSizeDefaultValueCreator.cs line since the bindable (StackLayout) is null since it's not a IFontElement

    static object FontSizeDefaultValueCreator(BindableObject bindable)
    {
        return ((IFontElement)bindable).FontSizeDefaultValueCreator();
    }

Steps to Reproduce

Create a Style in your App.XAML targeting a Label

            <Style
                x:Key="LabelNoteStyle" TargetType="Label">
                       <Setter Property="FontSize" Value="13"></Setter>
                       <Setter Property="TextColor" Value="{StaticResource BaseLightTextColor}"></Setter>
            </Style>

Apply this to a StackLayout
<StackLayout Grid.Row="0" Orientation="Horizontal" Style="{StaticResource LabelNoteStyle}">

Expected Behavior

Font info Is ignored

Actual Behavior

Crash in Anadroid and iOS in XF 3.6 and 4.x. It was hard to find as the error is just a null exception at the Main App level. I had to debug XF to find the actual bug.

[samhouts]

@ianvink Can you please attach a small project that demonstrates this issue? Thanks!

[ianvink]

BUG-6535.zip

Attached is a app that will crash. It's a File|New project with style info on the StackLayout of the first page.

Kaboom on:

static object FontSizeDefaultValueCreator(BindableObject bindable)
{
return ((IFontElement)bindable).FontSizeDefaultValueCreator();
}

[samhouts]

@StephaneDelcroix Do you think we should try to prevent the crash or give a better error message?

[ianvink]

My vote: it’s a Programmer error , Inform but don’t kill the app.

[andreinitescu]

@ianvink

My vote: it’s a Programmer error , Inform but don’t kill the app.

In my opinion, this must throw an exception, app really must terminate if exception is unhandled, you're setting an incompatible style. Failing silently is not a good idea, it hides issues in our apps which otherwise it would be extremely hard to track down.

I made a PR for this, see #6677

[StephaneDelcroix]

@StephaneDelcroix Do you think we should try to prevent the crash or give a better error message?

It should definitely NOT crash with a NRE or an InvalidCastException. We do have the TargetType, so we should validate. Then I would throw a meaningful exception at runtime, or compile time if possible.