[Core] Fix implicit conversion from null Uri to ImageSource

[kentcb]

Description of Change

Ensure the implicit conversion from Uri to ImageSource exhibits the same behavior as implicitly converting from string to ImageSource in the case where the input is null.

Bugs Fixed

Currently, implicitly converting a null Uri to an ImageSource is throwing a NullReferenceException. This PR fixes that.

API Changes

None

Behavioral Changes

No longer any need to work around the bug like this:

someImage.Source = someUri?.ToString();

One can now just do this:

someImage.Source = someUri;

PR Checklist

• Has tests (if omitted, state reason in description)
• Rebased on top of master at time of PR
• Changes adhere to coding standard
• Consolidate commits as makes sense

[kentcb]

This test (where the input is null) seemed to be completely missing, so I added it. Note that I am only asserting the existing behavior here - I don't actually agree with the behavior, but it is what it is.

[kentcb]

This is the companion test which was previously failing, but now passes. Again, I don't agree with the behavior, but I made it match the existing behavior for null strings.

[StephaneDelcroix]

Hi @kentcb,

congrats on your first PR ! 🎉

The PR looks right, but we should decide if the current behavior is right (and accept your fix) or wrong (and fix the conversion from null string so it returns a null ImageSource).

As you stated, that behavior wasn't tested, and that means no one really thought about it that much.

I'm leaning toward fixing the current behavior if the current renderers behave the same way with a null ImageSource and with a FileImageSource pointing to null (they probably should).

@jassmith could you provide your input here ? Thx

[kentcb]

@StephaneDelcroix yeah, as insinuated by my comments, I'd much rather the behavior of null translating to null - seems far more sensible to me. Just say the word and I'll update my PR accordingly.

[StephaneDelcroix]

@kentcb we had a quick discussion about this with the team. If a FileImageSource pointing to null behave the same on the renderers as a null ImageSource, it's better to go that route.

Could you please test that and update your patch? thanks

[kentcb]

@StephaneDelcroix I'm afraid I have nothing but questions and problems now...

1. Can you clarify? Are you saying we should try to convert null Uris and strings to a null ImageSource?
2. I can't seem to reference Xamarin.Forms as source. When I do, I get a bunch of errors because the XAML compiler is not run. Is there some trick to consuming XF as source?
3. I don't think I have the UWA tooling installed, so I may struggle to test there.

[StephaneDelcroix]

@kentcb

1. yes, I think all the op_implicit for ImageSource should return null if the input is null
2. you should <Import Project="..\.nuspec\Xamarin.Forms.targets" /> (use the right relative path). See https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Controls/Xamarin.Forms.Controls.csproj#L260 for example
3. don't need a full test. a manual test on iOS and android will be enough

[rmarinho]

Hi @kentcb can you rebase so we can run the build on CI, it has a expired certificate.

[jassmith]

Lets rebase this ourselves and merge it

[StephaneDelcroix]

should return null

[StephaneDelcroix]

manually merged