awesome-inc / FontAwesome.Sharp

A library for using Font Awesome in WPF & Windows Forms applications
Apache License 2.0
376 stars 87 forks source link

System.ArgumentException: '' is not a valid value for property 'FontFamily' #119

Closed hypetsch closed 4 months ago

hypetsch commented 4 months ago

Describe the bug Not sure if we are using the library wrong but applying an icon through style resources results in a System.ArgumentException in FontAwesome.Sharp.IconBlockBase.OnIconPropertyChanged

To Reproduce Created a sample app to reproduce the issue: https://github.com/hypetsch/FontAwesome.Sharp.TestApp

Expected behavior Style should be applied without exception.

Additional context Stacktrace of the exception:

   at System.Windows.DependencyObject.SetValueCommon(DependencyProperty dp, Object value, PropertyMetadata metadata, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType, Boolean isInternal)
   at System.Windows.DependencyObject.SetValue(DependencyProperty dp, Object value)
   at FontAwesome.Sharp.IconBlockBase`1.OnIconPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
   at System.Windows.FrameworkElement.OnPropertyChanged(DependencyPropertyChangedEventArgs e)
   at System.Windows.Controls.TextBlock.OnPropertyChanged(DependencyPropertyChangedEventArgs e)
   at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args)
   at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType)
   at System.Windows.DependencyObject.InvalidateProperty(DependencyProperty dp, Boolean preserveCurrentValue)
   at System.Windows.StyleHelper.InvalidatePropertiesOnTemplateNode(DependencyObject container, FrameworkObject child, Int32 childIndex, FrugalStructList`1& childRecordFromChildIndex, Boolean isDetach, FrameworkElementFactory templateRoot)
   at System.Windows.StyleHelper.ClearTemplateChain(HybridDictionary[] instanceData, FrameworkElement feContainer, FrameworkContentElement fceContainer, List`1 templateChain, FrameworkTemplate oldFrameworkTemplate)
   at System.Windows.StyleHelper.ClearGeneratedSubTree(HybridDictionary[] instanceData, FrameworkElement feContainer, FrameworkContentElement fceContainer, FrameworkTemplate oldFrameworkTemplate)
   at System.Windows.StyleHelper.DoTemplateInvalidations(FrameworkElement feContainer, FrameworkTemplate oldFrameworkTemplate)
   at System.Windows.Controls.Control.OnTemplateChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
   at System.Windows.FrameworkElement.OnPropertyChanged(DependencyPropertyChangedEventArgs e)
   at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args)
   at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType)
   at System.Windows.DependencyObject.InvalidateProperty(DependencyProperty dp, Boolean preserveCurrentValue)
   at System.Windows.StyleHelper.InvalidateContainerDependents(DependencyObject container, FrugalStructList`1& exclusionContainerDependents, FrugalStructList`1& oldContainerDependents, FrugalStructList`1& newContainerDependents)
   at System.Windows.StyleHelper.DoStyleInvalidations(FrameworkElement fe, FrameworkContentElement fce, Style oldStyle, Style newStyle)
   at System.Windows.StyleHelper.UpdateStyleCache(FrameworkElement fe, FrameworkContentElement fce, Style oldStyle, Style newStyle, Style& styleCache)
   at System.Windows.FrameworkElement.OnStyleChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
   at System.Windows.FrameworkElement.OnPropertyChanged(DependencyPropertyChangedEventArgs e)
   at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args)
   at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType)
   at System.Windows.DependencyObject.SetValueCommon(DependencyProperty dp, Object value, PropertyMetadata metadata, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType, Boolean isInternal)
   at System.Windows.DependencyObject.SetValue(DependencyProperty dp, Object value)
   at System.Windows.FrameworkElement.SetResourceReference(DependencyProperty dp, Object name)
   at TestApp.MainWindow.ButtonClicked() in C:\Users\schraffl\source\repos\TestApp\FontAwesome.Sharp.TesttApp\MainWindow.xaml.vb:line 4
   at TestApp.MainWindow._Lambda$__R8-1(Object a0, RoutedEventArgs a1)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
   at System.Windows.Controls.Primitives.ButtonBase.OnClick()
   at System.Windows.Controls.Button.OnClick()
   at System.Windows.Controls.Primitives.ButtonBase.OnMouseLeftButtonUp(MouseButtonEventArgs e)
   at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.ReRaiseEventAs(DependencyObject sender, RoutedEventArgs args, RoutedEvent newEvent)
   at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
   at System.Windows.UIElement.RaiseTrustedEvent(RoutedEventArgs args)
   at System.Windows.Input.InputManager.ProcessStagingArea()
   at System.Windows.Input.InputProviderSite.ReportInput(InputReport inputReport)
   at System.Windows.Interop.HwndMouseInputProvider.ReportInput(IntPtr hwnd, InputMode mode, Int32 timestamp, RawMouseActions actions, Int32 x, Int32 y, Int32 wheel)
   at System.Windows.Interop.HwndMouseInputProvider.FilterMessage(IntPtr hwnd, WindowMessage msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Interop.HwndSource.InputFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
   at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
   at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
   at System.Windows.Application.RunDispatcher(Object ignore)
   at System.Windows.Application.RunInternal(Window window)
   at TestApp.Application.Main()
mkoertgen commented 4 months ago

Hi @hypetsch, thanks for reporting.

As far as I understand, you are using a ControlTemplate within a Style.

The style is then used e.g. for a Label. I am not sure a Label is a ContentControl (which can have sub-elements).

The error message seems a bit like conflicting FontFamilies from the Label and the IconBlock.

I guess, for styling a label a more simpler way would be to use the "fa:ToText" markup extension, cf.

hypetsch commented 4 months ago

Hi,

thx for the quick response. After some quick tests it seems that the "ToText approach" fixes our problem, so many thanks for the hint.

...fyi, we are migrating a .Net Framework 4.8 app to .NET8 and therefore switched to your package by trying to keep the changes as small as possible. Btw, as far as we saw it the exception happens during style switch because of "the current style being removed and not the new one added"...

...of course I don't know your library in detail, just thought that it would maybe make sense to set the fontfamily on the OnIconPropertyChanged only if it is not empty. I would asume that this could avoid the exception.

Again, many thx & feel free to close the issue.

mkoertgen commented 4 months ago

@hypetsch great that it helped.

Wow, interesting. I just recently pushed some code changes for updating the package to net8 (did not release yet).

seanofw commented 3 months ago

FYI: This is still a bug. I've encountered it several times over the past few weeks, including today.

In one of my debugging sessions, I saw that the font-lookup mechanism (TypefaceFor()) produced bad results when the Icon property changed from a real value to None (i.e., default(IconChar)). I didn't have time to dig further, but I suspect that's the root cause of this bug, and that the style changes were only triggering that behavior.


I just managed to reproduce it with code that looks like this:

<fa:IconBlock Icon="{Binding SelectedUser.Icon, Mode=OneWay}" />

Importantly, the view model declares SelectedUser as nullable, and some things do set it to null:

public User? SelectedUser { ... }

When that property gets set to null — importantly, it doesn't start as null — FontAwesomeSharp then explodes with an empty font-family error, seemingly because Icon is then default(IconChar) and it doesn't know what to do with that.

I successfully fixed my local case by declaring a FallbackValue, which avoids the transition-to-default that seems to be the root cause:

<fa:IconBlock Icon="{Binding SelectedUser.Icon, Mode=OneWay, FallbackValue=CircleQuestion}" />

While that workaround does work, it's probably worthwhile for someone more familiar with the logic of it to dig into TypefaceFor() and figure out why it can't handle the default properly, but I think that's the actual cause of this bug.

Hope this helps!

mkoertgen commented 3 months ago

Hi @seanofw ,

Thank you for digging deeper and sharing your insight. 👍

Makes totally sense! default(IconChar) ~ enum-defaults always fall back to numerical zero which blow up then! At first glance I would suggest to return a "null-Image" in this case.

Feel free to reopen and/or submit a PR.

seanofw commented 3 months ago

I'm not entirely sure as to the expected behavior, but do you think the fix could be as simple as just changing this

    internal static Typeface TypefaceFor(IconChar iconChar)
    {
        return Orphans.Contains(iconChar) ? null : Typefaces.Find(iconChar.UniCode(), out _, out _);
    }

to

    internal static Typeface TypefaceFor(IconChar iconChar)
    {
        return Orphans.Contains(iconChar) ? Typefaces[0] : Typefaces.Find(iconChar.UniCode(), out _, out _);
    }

? That would change the semantics slightly, in that TypefaceFor() would no longer return null to indicate failure, but reading through the usages, maybe there's no actual need to indicate "failure" in this case: Having IconChar.None map to character code 0 of any of the actual fonts might be good enough: That character code is likely unmapped, so you'd end up with either nothing at all being displayed, or a missing-character rectangle, both of which would be more reasonable than a crash.

mkoertgen commented 3 months ago

Almost. I guess the right part can return "null", i.e. Typefaces.Find(...). One of the skipped out-vars will probably be a boolean indicating if found or similar. In any-case we can check the return value or shorten it like this

return (Orphans.Contains(iconChar) ? null : Typefaces.Find(iconChar.UniCode(), out _, out _)) ?? Typefaces[0];