PowerToys icon indicating copy to clipboard operation
PowerToys copied to clipboard

[Run Plugin] Settings crashes if Numberbox is empty

Open 8LWXpg opened this issue 3 months ago • 8 comments

Microsoft PowerToys version

0.80.1

Installation method

PowerToys auto-update, WinGet

Running as admin

Yes

Area(s) with issue?

PowerToys Run

Steps to reproduce

  • Install a plugin that has settings in Numberbox
  • Click Numberbox
  • Hit backspace
  • Hit enter
  • PowerToys settings crashes and exit

✔️ Expected Behavior

Fallback to default or NumberBoxMin.

❌ Actual Behavior

Crash and exit.

Other Software

No response

8LWXpg avatar May 06 '24 03:05 8LWXpg

Note: I'm unable to hit a breakpoint under UpdateSettings with the steps mentioned above. But once the Numberbox is set to any numeric value, I am able to reach the breakpoint as intended.

code as following:

public void UpdateSettings(PowerLauncherPluginSettings settings)
{
    _count = (int?)(settings?.AdditionalOptions?.FirstOrDefault(x => x.Key == Count)?.NumberValue) ?? 5;
}

8LWXpg avatar May 07 '24 06:05 8LWXpg

Okay this is strange.We set NumberBoxMax and NumberBoxMin to double.Max and double.Min. So theoretically it shouldn't accept the empty value.

@8LWXpg

  • What plugin do you use that produces the crash?
  • Can you please share a screen video?

htcfreek avatar May 07 '24 09:05 htcfreek

Made a minimum example

https://github.com/microsoft/PowerToys/assets/105704427/3a43f32c-d042-458b-b7b6-15d8f42f6a6a

8LWXpg avatar May 07 '24 10:05 8LWXpg

@8LWXpg The PluginAdditionalOptions allow to define min and max values for the number box setting. Can you try if explicitly definig them fixes the bug? (If yes I think we have a problem with the is Null check in the settings app code.)

And maybe a bugreport would help.


htcfreek avatar May 07 '24 13:05 htcfreek

I tested with following option, still has the same issue:

public IEnumerable<PluginAdditionalOption> AdditionalOptions =>
[
    new ()
    {
        PluginOptionType = PluginAdditionalOption.AdditionalOptionType.Numberbox,
        Key = Setting,
        DisplayLabel = "Test NumberBox",
        NumberValue = 5,
        NumberBoxMin = 2,
        NumberBoxMax = 10,
    },
];

8LWXpg avatar May 07 '24 13:05 8LWXpg

@8LWXpg Does the number box ignore the min and max settings? Because normally it should fallback to the min value.

htcfreek avatar May 07 '24 15:05 htcfreek

Works with numbers but not when the input box is empty.

https://github.com/microsoft/PowerToys/assets/105704427/d6c3dbd7-0986-4292-814c-f2ca49c09d1f

8LWXpg avatar May 07 '24 15:05 8LWXpg

I did some tests and created a test app. The following is happening when clearing the number box and pressing enter:

winui_bug_numberBox For the input number boxes the Minimum is set to 10 and the Maximum is set to 50.

This only happens if you clear the value. If the value resets because you type a number outside of min/max (e.g. 5000) everything is handled correctly.

@Aaron-Junker , @niels9001 It feels confusing that ...

  • the shown value in the input NumberBox (with min/max set) is different form the real value in the ViewModel variable.
  • the number box des not handle empty input correctly if bind against a double.

I think the best idea is to move the min/max validation into the ViewModel and also add a NaN validation there. Thoughts and ideas?


Explanation what happens:

If you clear the number box the value form the number box is empty as it is a normal text box with number features. That causes the view model variable to initialize with its default value (int) or beeing NaN (double). If the value of the view model variable is not between the minimum and maximum as defined in the NumberBox properties, the NumberBox shows the minimum or maximum value that is defined in XAML code instead of the view model variable value.

htcfreek avatar May 11 '24 18:05 htcfreek