[UWP] Prevent crash when using DateTime.MinValue for DatePicker #1056
Conversation
| catch(ArgumentOutOfRangeException) | ||
| { | ||
| // This will be thrown if you try to set MinYear to DateTime.MinValue when the UTC offset is positive | ||
| // because the time is out range once it's converted. In that case, specify the Kind as UTC |
There was a problem hiding this comment.
time is out range
typo, might confuse future dev
| // This will be thrown if you try to set MinYear to DateTime.MinValue when the UTC offset is positive | ||
| // because the time is out range once it's converted. In that case, specify the Kind as UTC | ||
| // so it doesn't get converted. | ||
| mindate = DateTime.SpecifyKind(mindate, DateTimeKind.Utc); |
There was a problem hiding this comment.
I'm sure there's a reason, but why don't we just always get the Date value and use that instead of adding a try catch and additional logic?
There was a problem hiding this comment.
We actually do do that in UpdateMaximumDate() and that was the first thing I tried. But Date just returns another DateTime with the Time set to 0:00 so in this case the crash still happens.
I think what we should do is always specify the incoming value as Utc and avoid the extra object from getting the Date. This would eliminate the try/catch and also prevent similar cases where the original date might get converted to the day before (or day after for MaxDate).
hartez
added
the
DO-NOT-MERGE-!!! 🛑
jimmgarrido
force-pushed
the
uwp_min_date
branch
from
62b992e
to
8c32993
Compare
| @@ -90,13 +90,25 @@ void UpdateDate(DateTime date) | |||
| void UpdateMaximumDate() | |||
| { | |||
| DateTime maxdate = Element.MaximumDate; | |||
| Control.MaxYear = new DateTimeOffset(maxdate.Date); | |||
There was a problem hiding this comment.
Date just returns another DateTime object with the same date so it's redundant.
| { | ||
| Control.MinYear = new DateTimeOffset(mindate); | ||
| } | ||
| catch (ArgumentOutOfRangeException) |
There was a problem hiding this comment.
Turns out a try/catch is really just the simplest fix because the exception should only be thrown in this one scenario.
|
This should be okay to merge now. I didn't make any changes other than making the comment more clearer because this seems like an edge case so most people probably won't hit the logic inside the catch. |
samhouts
added
D15.5
and removed
cla-already-signed
DO-NOT-MERGE-!!! 🛑
Description of Change
On UWP, if you try to set the DatePicker.MinimumDate to DateTime.MinValue when the UTC offset is positive an
ArgumentOutOfRangeExceptionis thrown because the time gets converted to UTC which makes it go out of range.This fix catches the exception and specifies the date as UTC in order to prevent it from being converted.
Bugs Fixed
API Changes
None
Behavioral Changes
None
PR Checklist