-
Couldn't load subscription status.
- Fork 62
Screen Capture Requested #3655
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.
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
base: main
Are you sure you want to change the base?
Screen Capture Requested #3655
Changes from 5 commits
8f9fb8c
3b4a194
99ee6b4
52b4a00
b0fce82
4ec2410
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,357 @@ | ||||||||
| # Background | ||||||||
|
|
||||||||
| The HTML DOM's Screen Capture API `navigator.mediaDevices.getDisplayMedia` allows developers to | ||||||||
| get a video stream of a user's tabs, windows, or desktop. This API is available in WebView2, | ||||||||
| but the current default UI has some problems that we need to fix to make sure that hybrid apps | ||||||||
| using WebView2 have a more seamless web/native experience. This includes removing the tab column | ||||||||
| in the UI, replacing default strings and icons that do not match in WV2, and potentially having | ||||||||
| the ability to customize the UI itself. | ||||||||
|
|
||||||||
| These apps also expect that the screen capture dialog has an event before the UI is shown to give | ||||||||
| the host app an opportunity to block or allow UI from showing at all. | ||||||||
|
|
||||||||
| In this document we describe the updated API. We'd appreciate your feedback. | ||||||||
|
|
||||||||
| # Description | ||||||||
|
|
||||||||
| We propose introducing the `ScreenCaptureRequested` event. This event will be raised whenever | ||||||||
| the WebView2 and/or iframe corresponding to the CoreWebView2Frame or any of its descendant iframes | ||||||||
| requests permission to use the Screen Capture API before the UI is shown. | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the name "Requested", I was expecting that this was asking the host to do something. E.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, should we more align the name with getDisplayMedia? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's rename this to align with the other ..Starting events
|
||||||||
|
|
||||||||
| For convenience of the end developer, by default we plan to raise | ||||||||
| `ScreenCaptureRequested` on both `CoreWebView2Frame` and `CoreWebView2`. The | ||||||||
| `CoreWebView2Frame` event handlers will be invoked first, | ||||||||
| before the `CoreWebView2` event handlers. If `Handled` is set true as part of | ||||||||
| the `CoreWebView2Frame` event handlers, then the `ScreenCaptureRequested` event | ||||||||
| will not be raised on the `CoreWebView2`, and its event handlers will not be | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume that if I set args.Cancel=True for CoreWebView2Frame.ScreenCaptureRequested but don't mark it as handled, when CoreWebView2.ScreenCaptureRequested is raised, the args.Cancel property will return true? If so, it would be worth explicitly stating so in the doc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Explicitly note in docs: if you don't mark handled true then the same event args (with modifications) bubble up to the next event handler on the corewebview2 |
||||||||
| invoked. | ||||||||
|
||||||||
|
|
||||||||
| In the case of a nested iframe requesting permission, we will raise the event | ||||||||
| off of the top level iframe. | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nested frames are weird in the WebView2 API surface.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dave: Follow up with how we want events to behave wrt frames in the future. Plus public document on event patterns in WebView2. |
||||||||
|
|
||||||||
| # Examples | ||||||||
| ## C++: Registering Screen Requested Handler on CoreWebView2 | ||||||||
| ``` cpp | ||||||||
| wil::com_ptr<ICoreWebView2> m_webviewEventSource; | ||||||||
| EventRegistrationToken m_screenCaptureRequestedToken = {}; | ||||||||
|
|
||||||||
| m_webviewEventSource->add_ScreenCaptureRequested( | ||||||||
|
||||||||
| Callback<ICoreWebView2ScreenCaptureRequestedEventHandler>( | ||||||||
| [this](ICoreWebView2* sender, ICoreWebView2ScreenCaptureRequestedEventArgs* args) | ||||||||
| -> HRESULT | ||||||||
| { | ||||||||
| // Get Frame Info | ||||||||
| wil::com_ptr<ICoreWebView2FrameInfo> frameInfo; | ||||||||
| CHECK_FAILURE(args->get_FrameInfo(&frameInfo)); | ||||||||
|
||||||||
|
|
||||||||
| // Frame Source | ||||||||
| wil::unique_cotaskmem_string frameSource; | ||||||||
| CHECK_FAILURE(frameInfo->get_Source(&frameSource)); | ||||||||
|
|
||||||||
| // If the host app wants to cancel the request for a specific source | ||||||||
| if (frameSource = "https://developer.microsoft.com/en-us/microsoft-edge/webview2/") | ||||||||
|
||||||||
| if (frameSource = "https://developer.microsoft.com/en-us/microsoft-edge/webview2/") | |
| if (frameSource == "https://developer.microsoft.com/en-us/microsoft-edge/webview2/") | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please fix.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a safe comparison? The trailing "/" is guaranteed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reuse other sample code that does URI comparison. Also compare just hostname.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After we set Handled = true, what does it mean when Cancel is left as its default value of false?
Does it mean "Don't cancel, show default UI"?
Does it mean "Don't cancel, allow silent capture without any prompt"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it mean "Don't cancel, show default UI"?
This is what happens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if I set Cancel = true but don't set Handled? Does the next handler observe Cancel = true? Or does each handler get a fresh Cancel = false?
What happens if nobody sets Handled = true, but somebody sets Cancel = true? Does that cancel? Or do you have to Handle the event in order to Cancel it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed above
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a C#
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a similar one below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please fix.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent variable name 'webView'/'m_webView'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar issue to above with member / local naming and logic. Please fix similar to above.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the remaining frame handlers from being invoked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text is currently correct. It will need to be updated in the future when we support grand+child frames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have this pattern anywhere else? Generally when you take a deferral, you can set results later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave this as is. We do have this pattern elsewhere in WebView2.
Dave: Include this in how event bubbling should work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Although if you don't set Handled, someone downstream might clear your Cancel)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Please add this to the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is talking about UI that is never shown in the spec, so it's not very helpful. But really, the second half of the paragraph sounds like inside baseball. Maybe shorten to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking my understanding ... This API isn't about enabling the host to make the experience more seamless (replacing default strings and such), it's just about the ability to cancel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next time we'll separate out back story and future plans from rest of background.