dotnet
Clipboard.ContainsImage inconsistent with Clipboard.GetImage
In #493 it was decided that removing metafile support from Clipboard.GetImage is by design. However Clipboard.SetImage and Clipboard.ContainsImage were never adjusted and still support metafiles, this leads to an inconsistent API.
Clipboard.SetImage(new Metafile("example.emf")); // copies the metafile to the clipboard
Clipboard.ContainsImage(); // returns true
Clipboard.GetImage(); // returns null by design, issue #493
GetImagereturning null (instead of throwing an exception) is correct, since all clipboard APIs return null if there is no compatible data available. Since #493 removes compatibility for reading metafiles this is expected behavior.ContainsImagereturningtrueif a metafile is present is a bug, it must returnfalsebecauseGetImagewill not be able to read the metafile.SetImagestill supports metafiles so we now have a write-only API which is arguably not a bug, but it may be confusing why you can write metafiles but not read them back. If you want to drop support for metafiles from SetImage as well then it should throw an exception.
Side note: I didn't test it explicitely but I assume the DataObject APIs behave the same way as the Clipboard APIs and need the same fix.
adding @Tanya-Solyanik and @merriemcgaw to discuss further on the experience here.
@RussKie @merriemcgaw is it too late to reclassify this for 5.0 and have the API reviewed ?
While this is tagged "documentation" it is unclear of whether this needs API changes, some APIs still support metafiles while other APIs don't. It would be nice to have the API consistent and if there are any changes necessary it probably should be done for 5.0 if possible.
If API/implementation changes are needed I can make a PR.
If its too late to make it for 5.0 I understand, just thought I'd ask, since recent issue #3025 reminded me that there is an open issue with this API.
is it too late to reclassify this for 5.0 and have the API reviewed ?
No, there is still time :) Thank you for keeping track.
GetImagereturning null (instead of throwing an exception) is correct, since all clipboard APIs return null if there is no compatible data available. Since #493 removes compatibility for reading metafiles this is expected behavior.
👍
ContainsImagereturningtrueif a metafile is present is a bug, it must returnfalsebecauseGetImagewill not be able to read the metafile.
Agree, the right thing to do would be to return false.
SetImagestill supports metafiles so we now have a write-only API which is arguably not a bug, but it may be confusing why you can write metafiles but not read them back. If you want to drop support for metafiles from SetImage as well then it should throw an exception.
Agree as well, it sounds sensible to throw. @JeremyKuhne thoughts?
If API/implementation changes are needed I can make a PR.
👍
Should I be adding these changes to PR #3039 or do you want the API changes separated from the process crash fix?
Do you mind keeping them separate just to avoid confusion in the future?
Not at all, will create a separate PR for the DataObject changes not directly related to the process crash.
Not at all, will create a separate PR for the DataObject changes not directly related to the process crash.
I kinda forgot about writing a response for why there is no PR. It turned out that ContainsImage cannot easily differentiate what kind of image is on the clipboard (metafiles claim they are System.Drawing.Bitmap !), so if its a metafile ContainsImage has to say true because it might just as well be a real bitmap - just can't tell the difference before attempting deserializing it (and ContainsImage should not attempt deserialization, its supposed to be a cheap operation).
As far as this issue is concerned I believe there is nothing actionable beyond documenting the deprecation of metafile support on clipboard and drag'n'drop (for security reasons I believe?). If thats done already I think this can be closed.
As far as this issue is concerned I believe there is nothing actionable beyond documenting the deprecation of metafile support on clipboard and drag'n'drop (for security reasons I believe?). If thats done already I think this can be closed.
@merriemcgaw I guess that means we can close this. Since nothing is to be done and nothing is changed?
I'd like @JeremyKuhne and @lonitra to factor this into their research and planning for resources and clipboard moving forward. BinaryFormatter deprecation and how we decide to handle that might open up some options here.