microsoft / microsoft/WindowsAppSDK
FrameworkUDK's NotificationProperties::get_Payload should return a wstring
@robertzhou-wpn is already working on this.
Since Aug 31, 2022.
- Dominant language
- C++
- Stars
- 4.7k
- Forks
- 471
- Avg merge
- 3d 13h
- Merged PRs (30d)
- 28
Description
### Describe the bug
In the FrameworkUDK, NotificationProperties' ctor takes a PCWSTR and internally converts it to utf8 but, the corresponding get_Payload() operation returns a utf8 string. This means that NotificationProperties is responsible for encoding the payload while the caller is responsible for decoding it. This not only breaks the symmetry developers are usually expecting from this king of API, it means the code responsible for the encoding is located in an entirely different part of the system (in this case in different DLLs and different repos), this is a recipe for disaster as there is a risk for the encoding and decoding functions to diverge.
Consider fixing get_Payload(), so it returns a wide string, if get_Payload() can't be changed, consider adding an overload that would return a wstring.
Then remove the decoding function from WindowsAppSDK.
### Steps to reproduce the bug
This decoding function wil::unique_cotaskmem_string Microsoft::Windows::AppNotifications::Helpers::ConvertUtf8StringToWideString(unsigned long length, const byte* utf8String), should be in the UDK, next to the encoding one.
### Expected behavior
_No response_
### Screenshots
_No response_
### NuGet package version
_No response_
### Packaging type
_No response_
### Windows version
_No response_
### IDE
_No response_
### Additional context
_No response_
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.