Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Review: Web Notification #3186

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

API Review: Web Notification #3186

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1c1d3d0
Add Web Notification API Spec draft
peiche-jessica Dec 9, 2022
952a757
Apply suggestions from code review
peiche-jessica Dec 13, 2022
e03a214
Address initial feedback part 1
peiche-jessica Dec 13, 2022
cbd9d61
Add enum CoreWebView2TextDirectionKinds to c# API details
peiche-jessica Dec 13, 2022
5e22e5f
Report* and CloseRequested
peiche-jessica Jan 4, 2023
0b9a618
Persistent -> ProfileNotificationReceivedEventHandler
peiche-jessica Jan 6, 2023
61d7ae7
Address initial feedback part 2
peiche-jessica Jan 6, 2023
c61f653
Update ref doc
peiche-jessica Jan 6, 2023
b8395cb
Add DataAsJson and TryGetDataAsString
peiche-jessica Jan 9, 2023
88418d0
Add links to Notification.actions
peiche-jessica Jan 9, 2023
0b6a3aa
Apply API name suggestion
peiche-jessica Jan 10, 2023
25207b4
Clarify Handled and ReportXXX methods
peiche-jessica Jan 10, 2023
8ef88d3
Clarify deferral info
peiche-jessica Jan 10, 2023
4908aaf
Fix sample code
peiche-jessica Jan 10, 2023
993fe9f
Make ReportClicked and ReportClosed async
peiche-jessica Jan 10, 2023
320baf0
Address feedback
peiche-jessica Feb 1, 2023
16ba2bc
Update sample
peiche-jessica Feb 3, 2023
4493c00
Merge pull request #3049 from MicrosoftEdge/api-notification-draft
peiche-jessica Feb 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address initial feedback part 1
  • Loading branch information
@peiche-jessica
peiche-jessica committed Dec 13, 2022
commit e03a2149bdc0fd85cace9cc89f05d7aed1b6a21f
129 changes: 65 additions & 64 deletions specs/WebNotification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,17 @@ The `NotificationReceived` events are raised on `CorebWebView2` and
`CoreWebView2Profile` object respectively for non-persistent and persistent
notifications respectively.

The `NotificationReceived` event on `CoreWebView2` and `CoreWebView2Profile` let you intercept the web non-persistent and persistent notifications. The host can use the `Notification` property on the `NotificationReceivedEventArgs` to construct an notification matching the look and feel of the host app. The host can also use such information to decide to show or not show a particular notification. The host can `GetDeferral` or set the `Handled` property on the `NotificationReceivedEventArgs` to handle the event at a later time or let WebView2 know if the notification has been handled. By default, if the `NotificationReceived` event is not handled by the host, the web notification will be displayed using the default notification UI provided by WebView2 Runtime.
The `NotificationReceived` event on `CoreWebView2` and `CoreWebView2Profile` let
you intercept the web non-persistent and persistent notifications. The host can
use the `Notification` property on the `NotificationReceivedEventArgs` to
construct an notification matching the look and feel of the host app. The host
can also use such information to decide to show or not show a particular
notification. The host can `GetDeferral` or set the `Handled` property on the
`NotificationReceivedEventArgs` to handle the event at a later time or let
WebView2 know if the notification has been handled. By default, if the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the Handled property applies only to permission requests from frames. (Handling the event prevents the event from propagating to the top-level document.) "Handling" the event from the top-level document has no effect.

This is sort of hinted at in the documentation but not explicitly stated.

The host may set this flag to TRUE to prevent the PermissionRequested event from firing on the CoreWebView2 as well.

So it says that this prevents the event from propagating to the root document. But it doesn't say that it has no effect if fired on the CoreWebView2 itself.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update wording.

`NotificationReceived` event is not handled by the host, the web notification
will be displayed using the default notification UI provided by WebView2
Runtime.

# Examples

Expand Down Expand Up @@ -251,28 +261,16 @@ void SettingsComponent::ShowNotification(
```cpp
/// Specifies the text direction of the notification.
[v1_enum]
typedef enum COREWEBVIEW2_NOTIFICATION_DIRECTION_KINDS {
typedef enum COREWEBVIEW2_TEXT_DIRECTION_KINDS {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Singular, since this is not a flags enum.

Suggested change
typedef enum COREWEBVIEW2_TEXT_DIRECTION_KINDS {
typedef enum COREWEBVIEW2_TEXT_DIRECTION_KIND {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a flags enum so should be singular (...KIND)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix.

/// Indicates that the notification text direction adopts the browser's language setting behavior.
COREWEBVIEW2_NOTIFICATION_DIRECTION_KINDS_AUTO,
COREWEBVIEW2_TEXT_DIRECTION_KINDS_DEFAULT,

/// Indicates that the notification text is left-to-right.
COREWEBVIEW2_NOTIFICATION_DIRECTION_KINDS_LTR,
COREWEBVIEW2_TEXT_DIRECTION_KINDS_LEFT_TO_RIGHT,

/// Indicates that the notification text is right-to-left.
COREWEBVIEW2_NOTIFICATION_DIRECTION_KINDS_RTL,
} COREWEBVIEW2_NOTIFICATION_DIRECTION_KINDS;

/// This is the notification action for interacting with the notification.
typedef struct COREWEBVIEW2_NOTIFICATION_ACTION {
/// A string identifying a user action to be displayed on the notification.
LPWSTR Action;

/// A string containing action text to be shown to the user.
LPWSTR Title;

/// A string containing the URL of an icon to display with the action.
LPWSTR Icon;
} COREWEBVIEW2_NOTIFICATION_ACTION;
COREWEBVIEW2_TEXT_DIRECTION_KINDS_RIGHT_TO_LEFT,
} COREWEBVIEW2_TEXT_DIRECTION_KINDS;

/// This is the ICoreWebView2Profile3 interface that manages WebView2 Web
/// Notification functionality.
Expand Down Expand Up @@ -334,7 +332,7 @@ interface ICoreWebView2NotificationReceivedEventArgs : IUnknown {

/// The notification that was received. End developers can access the
/// properties on the Notification object to show their own notification.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar. "You can access" to show "their own notification".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix.

[propget] HRESULT Notification([out, retval] ICoreWebView2Notification** notification);
[propget] HRESULT Notification([out, retval] ICoreWebView2Notification** value);

/// Sets whether the `NotificationReceived` event is handled by the host after
/// the event handler completes or if there is a deferral then after the
Expand Down Expand Up @@ -367,24 +365,37 @@ interface ICoreWebView2NotificationClosedEventHandler : IUnknown {
[in] IUnknown* args);
}

/// This is the notification for interacting with the notification.
[uuid(07DD3067-2B86-47F6-AB96-D74825C2DA41), object, pointer_default(unique)]
interface ICoreWebView2NotificationAction : IUnknown {
/// A string identifying a user action to be displayed on the notification.
[propget] HRESULT Action([out, retval] LPWSTR* value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that Action has a property called Action, but that's what the DOM API calls it, so I'm inclined to match it.

It's not clear what the difference between Action and Title is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change required.


/// A string containing action text to be shown to the user.
[propget] HRESULT Title([out, retval] LPWSTR* value);

/// A string containing the URI of an icon to display with the action.
[propget] HRESULT IconUri([out, retval] LPWSTR* value);
}

/// A collection of notification actions.
[uuid(89D8907E-18C7-458B-A970-F91F645E4C43), object, pointer_default(unique)]
interface ICoreWebView2NotificationActionCollection : IUnknown {
interface ICoreWebView2NotificationActionCollectionView : IUnknown {
/// The number of notification actions contained in the
/// ICoreWebView2NotificationActionCollection.
[propget] HRESULT Count([out, retval] UINT* count);
/// ICoreWebView2NotificationActionCollectionView.
[propget] HRESULT Count([out, retval] UINT* value);

/// Gets the notification action at the given index.
HRESULT GetValueAtIndex([in] UINT index,
[out, retval] COREWEBVIEW2_NOTIFICATION_ACTION* value);
[out, retval] ICoreWebView2NotificationAction** value);
}

/// A collection of unsigned long integers.
[uuid(974A91F8-A309-4376-BC70-7537D686533B), object, pointer_default(unique)]
interface ICoreWebView2UnsignedLongCollection : IUnknown {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ICoreWebView2UnsignedLongCollectionView, since it's read-only?

"UInt32" instead of "UnsignedLong" to be implementation-independent? (C# and some C++ compilers use 64-bit longs.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change required. See below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this rather than an array?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be an array instead.

/// The number of unsigned long integers contained in the
/// ICoreWebView2UnsignedLongCollection.
[propget] HRESULT Count([out, retval] UINT* count);
[propget] HRESULT Count([out, retval] UINT* value);

/// Gets the unsigned long integer at the given index.
HRESULT GetValueAtIndex([in] UINT index, [out, retval] UINT64* value);
Expand Down Expand Up @@ -418,7 +429,7 @@ interface ICoreWebView2Notification : IUnknown {

/// The host may run this to report the persistent notification has been
/// activated with a given action. The action index corresponds to the index
/// in NotificationActionCollection. This returns `E_INVALIDARG` if an invalid
/// in NotificationActionCollectionView. This returns `E_INVALIDARG` if an invalid
/// action index is provided. Use `Click` to activate an non-persistent
/// notification. This will no-op if `Show` is not run or `Close` has been
/// run.
Expand All @@ -434,17 +445,17 @@ interface ICoreWebView2Notification : IUnknown {
///
/// The caller must free the returned string with `CoTaskMemFree`. See
/// [API Conventions](/microsoft-edge/webview2/concepts/win32-api-conventions#strings).
[propget] HRESULT Body ([out, retval] LPWSTR* body);
[propget] HRESULT Body([out, retval] LPWSTR* value);

/// Returns an IDataObject that represents a structured clone of the
/// notification's data.
/// Returns `null` if the optional Notification property does not exist.
[propget] HRESULT Data ([out, retval] IDataObject** data);
[propget] HRESULT Data([out, retval] IDataObject** value);

/// The text direction of the notification as specified in the constructor's
/// options parameter.
/// The default value is `COREWEBVIEW2_NOTIFICATION_DIRECTION_KINDS_AUTO`.
[propget] HRESULT Direction ([out, retval] COREWEBVIEW2_NOTIFICATION_DIRECTION_KINDS* value);
/// The default value is `COREWEBVIEW2_TEXT_DIRECTION_KINDS_DEFAULT`.
[propget] HRESULT Direction([out, retval] COREWEBVIEW2_TEXT_DIRECTION_KINDS* value);

/// The language code of the notification as specified in the constructor's
/// options parameter. It is in the format of
Expand All @@ -455,75 +466,75 @@ interface ICoreWebView2Notification : IUnknown {
///
/// The caller must free the returned string with `CoTaskMemFree`. See
/// [API Conventions](/microsoft-edge/webview2/concepts/win32-api-conventions#strings).
[propget] HRESULT Language ([out, retval] LPWSTR* language);
[propget] HRESULT Language([out, retval] LPWSTR* value);

/// The ID of the notification (if any) as specified in the constructor's
/// options parameter.
/// The default value is an empty string.
///
/// The caller must free the returned string with `CoTaskMemFree`. See
/// [API Conventions](/microsoft-edge/webview2/concepts/win32-api-conventions#strings).
[propget] HRESULT Tag ([out, retval] LPWSTR* tag);
[propget] HRESULT Tag([out, retval] LPWSTR* value);

/// The URL of the image used as an icon of the notification as specified in
/// The URI of the image used as an icon of the notification as specified in
/// the constructor's options parameter.
/// Returns `null` if the optional Notification property does not exist.
///
/// The caller must free the returned string with `CoTaskMemFree`. See
/// [API Conventions](/microsoft-edge/webview2/concepts/win32-api-conventions#strings).
[propget] HRESULT Icon ([out, retval] LPWSTR* icon);
[propget] HRESULT IconUri([out, retval] LPWSTR* value);

/// The title of the notification as specified in the first parameter of the
/// constructor.
///
/// The caller must free the returned string with `CoTaskMemFree`. See
/// [API Conventions](/microsoft-edge/webview2/concepts/win32-api-conventions#strings).
[propget] HRESULT Title([out, retval] LPWSTR* title);
[propget] HRESULT Title([out, retval] LPWSTR* value);


/// The actions available for users to choose from for interacting with the
/// notification. Note that actions are only supported for persistent notifications.
/// Returns `null` if the optional Notification property does not exist.
[propget] HRESULT Actions([out, retval] ICoreWebView2NotificationActionCollection** actions);
/// An empty NotificationActionCollectionView is returned if no notification actions.
[propget] HRESULT Actions([out, retval] ICoreWebView2NotificationActionCollectionView** value);

/// The URL of the image used to represent the notification when there is not
/// The URI of the image used to represent the notification when there is not
/// enough space to display the notification itself.
/// Returns `null` if the optional Notification property does not exist.
///
/// The caller must free the returned string with `CoTaskMemFree`. See
/// [API Conventions](/microsoft-edge/webview2/concepts/win32-api-conventions#strings).
[propget] HRESULT Badge([out, retval] LPWSTR* badge);
[propget] HRESULT BadgeUri([out, retval] LPWSTR* value);

/// The URL of an image to be displayed as part of the notification.
/// The URI of an image to be displayed as part of the notification.
/// Returns `null` if the optional Notification property does not exist.
///
/// The caller must free the returned string with `CoTaskMemFree`. See
/// [API Conventions](/microsoft-edge/webview2/concepts/win32-api-conventions#strings).
[propget] HRESULT Image([out, retval] LPWSTR* image);
[propget] HRESULT ImageUri([out, retval] LPWSTR* value);

/// Specifies whether the user should be notified after a new notification
/// replaces an old one.
/// The default value is `FALSE`.
[propget] HRESULT Renotify([out, retval] BOOL* renotify);
[propget] HRESULT Renotify([out, retval] BOOL* value);

/// A boolean value indicating that a notification should remain active until
/// the user clicks or dismisses it, rather than closing automatically.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think OS notifications support this. Is this a requirement of the DOM API, or can hosts fudge it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update docs to note that you may not be able to necessarily implement this.

/// The default value is `FALSE`.
[propget] HRESULT RequireInteraction([out, retval] BOOL* requireInteraction);
[propget] HRESULT RequireInteraction([out, retval] BOOL* value);

/// Specifies whether the notification should be silent — i.e., no sounds or
/// vibrations should be issued, regardless of the device settings.
/// The default value is `FALSE`.
[propget] HRESULT Silent([out, retval] BOOL* silent);
[propget] HRESULT Silent([out, retval] BOOL* value);

/// Specifies the time at which a notification is created or applicable (past,
/// present, or future).
/// Returns `null` if the optional Notification property does not exist.
[propget] HRESULT Timestamp([out, retval] double* timestamp);
[propget] HRESULT Timestamp([out, retval] double* value);

/// Specifies a vibration pattern for devices with vibration hardware to emit.
/// Returns `null` if the optional Notification property does not exist.
[propget] HRESULT Vibrate([out, retval] ICoreWebView2UnsignedLongCollection** vibrationPattern);
[propget] HRESULT Vibrate([out, retval] ICoreWebView2UnsignedLongCollection** value);
}
```

Expand All @@ -532,20 +543,13 @@ namespace Microsoft.Web.WebView2.Core
{
runtimeclass CoreWebView2NotificationReceivedEventArgs;
runtimeclass CoreWebView2Notification;
struct CoreWebView2NotificationAction
runtimeclass CoreWebView2NotificationAction
{
String Action;
String title;
String Icon;

};
struct CoreWebView2NotificationAction
{
String Action;
String title;
String Icon;

};
// ICoreWebView2NotificationAction members
String Action { get; };
String Title { get; };
String IconUri { get; };
}
runtimeclass CoreWebView2Profile
{
...
Expand All @@ -563,9 +567,6 @@ namespace Microsoft.Web.WebView2.Core
{
// ICoreWebView2_17 members
event Windows.Foundation.TypedEventHandler<CoreWebView2, CoreWebView2NotificationReceivedEventArgs> NotificationReceived;



}
...
}
Expand All @@ -585,11 +586,11 @@ namespace Microsoft.Web.WebView2.Core
CoreWebView2NotificationDirectionKinds Direction { get; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this meant to be the text-reading direction? (CoreWebView2TextDirectionKinds)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Please ensure this is updated after rerunning tool to generate MIDL3.

String Language { get; };
String Tag { get; };
String Icon { get; };
String IconUri { get; };
String Title { get; };
IVectorView<CoreWebView2NotificationAction> Actions { get; };
String Badge { get; };
String Image { get; };
String BadgeUri { get; };
String ImageUri { get; };
Boolean Renotify { get; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure all the MIDL3 matches the COM IDL after running gen tool.

Boolean RequireInteraction { get; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Boolean RequireInteraction { get; };
Boolean RequiresInteraction { get; };

Boolean Silent { get; };
Expand Down