microsoft / microsoft/WindowsAppSDK

FrameworkUDK's NotificationProperties::get_Payload should return a wstring

Open
#2,208 0 comments 0 reactions 2 assignees View on GitHub

@robertzhou-wpn is already working on this.

Since Aug 31, 2022.

area-Notifications feature proposal good first issue
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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.