Screen Capture Requested #3655

srwei · 2023-07-20T23:55:11Z

No description provided.

…nd updated some documentation

ScreenCaptureRequested draft

mikeduglas · 2023-07-21T08:03:39Z

specs/ScreenCaptureRequested.md

+webView.CoreWebView2.ScreenCaptureRequested += (sender, screenCaptureArgs) =>
+{   
+    // Get Frame Info
+    wil::com_ptr<ICoreWebView2FrameInfo> frameInfo;


It is not a C#

There's a similar one below

Yes please fix.

oldnewthing · 2023-07-21T17:29:45Z

specs/ScreenCaptureRequested.md

+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 


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

This API is available in WebView2 and displays a default picker which may not be appropriate for all apps.

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?

Next time we'll separate out back story and future plans from rest of background.

oldnewthing · 2023-07-21T17:43:13Z

specs/ScreenCaptureRequested.md

+invoked.
+
+In the case of a nested iframe requesting permission, we will raise the event
+off of the top level iframe.


Nested frames are weird in the WebView2 API surface.

Sometimes actions related to nested frames result in the event being raised on the nested frame. (DOMContentLoaded, navigation events, WebMessageReceived.)

Sometimes actions related to nested frames result in the event being raised on the top level frame + the root document. (PermissionRequested, ScreenCaptureRequested.)

Dave: Follow up with how we want events to behave wrt frames in the future. Plus public document on event patterns in WebView2.

oldnewthing · 2023-07-21T17:49:28Z

specs/ScreenCaptureRequested.md

+                                    // CoreWebView2Frame event handler, then we will not raise the
+                                    // ScreenCaptureRequested event off the CoreWebView2.
+
+                                    CHECK_FAILURE(args->put_Handled(true));


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"?

Does it mean "Don't cancel, show default UI"?

This is what happens

oldnewthing · 2023-07-21T17:51:14Z

specs/ScreenCaptureRequested.md

+                                    // ScreenCaptureRequested event off the CoreWebView2.
+
+                                    CHECK_FAILURE(args->put_Handled(true));
+                                    return S_OK;


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?

Discussed above

MikeHillberg · 2023-07-24T17:33:00Z

specs/ScreenCaptureRequested.md

+
+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. 


From the name "Requested", I was expecting that this was asking the host to do something. E.g. BasicAuthenticationRequested is asking the host for a response. Really, I think this event is saying "I'm about to start the screen capture process, this is your chance to reject it"? That suggests a name more like ScreenCaptureStarting.

Also, should we more align the name with getDisplayMedia?

Let's rename this to align with the other ..Starting events

ScreenCaptureStarting

MikeHillberg · 2023-07-24T17:34:49Z

specs/ScreenCaptureRequested.md

+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
+invoked.


Are there really 4 cases that need to be supported (the combinations of Handled and Cancel)? I think if you set Cancel you're always going to also set Handled. And if you don't set Cancel I think you're not going to set Handled.

I.e., I don't think Handled is necessary; if Cancel is set, then stop raising events.

On the other hand, the double boolean is clearer that one of them controls propagation and the other controls the system behavior. It also aligns with other uses of Handled and Cancel.

You might set Handled=true and Cancel=false to mean "Trust me, it's okay. Don't need to ask others for permission to capture. I give you permission to capture."

And you might set Handled=false and Cancel=true to mean "I think you should cancel, but call the other handlers because one of them might have a stronger opinion."

Leave as is: match existing xaml / dom patterns of bubbling

kmahone · 2023-07-25T17:28:08Z

specs/ScreenCaptureRequested.md

+wil::com_ptr<ICoreWebView2> m_webviewEventSource;
+EventRegistrationToken m_screenCaptureRequestedToken = {};
+
+m_webviewEventSource->add_ScreenCaptureRequested(


This should just be m_webView or something similar. It is not an EventSource.

Yes please rename. Also its named like a member but looks like a local. Probably needs to be updated to fix that as well.

kmahone · 2023-07-25T17:33:39Z

specs/ScreenCaptureRequested.md

+```c#
+private WebView2 webView;
+
+m_webview.CoreWebView2.FrameCreated += (sender, frameCreatedArgs) =>


Inconsistent variable name 'webView'/'m_webView'.

similar issue to above with member / local naming and logic. Please fix similar to above.

kmahone · 2023-07-25T18:03:06Z

specs/ScreenCaptureRequested.md

+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
+invoked.


Assuming nothing handles the event on the CoreWebView2Frame, we will raise the event on the CoreWebView2. The two events share the same args, so the CoreWebView2 event args has a Handled property. What does Handled do in this context?

It does nothing in this case. We should explicitly document that is doing nothing in that case.

Dave: Follow up with how we want Handled to behave in this sort of case in the future.

vickiez · 2023-07-25T18:04:43Z

specs/ScreenCaptureRequested.md

+            { 
+                // Get Frame Info
+                wil::com_ptr<ICoreWebView2FrameInfo> frameInfo;
+                CHECK_FAILURE(args->get_FrameInfo(&frameInfo));


Should this be OriginalSourceFrameInfo? + other places

Yes please fix.

kmahone · 2023-07-25T18:07:59Z

specs/ScreenCaptureRequested.md

+`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


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.

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

MikeHillberg · 2023-07-25T19:32:49Z

specs/ScreenCaptureRequested.md

+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 


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?

MikeHillberg · 2023-07-25T19:38:16Z

specs/ScreenCaptureRequested.md

+                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/") 


Suggested change

if (frameSource = "https://developer.microsoft.com/en-us/microsoft-edge/webview2/")

if (frameSource == "https://developer.microsoft.com/en-us/microsoft-edge/webview2/")

Yes please fix.

MikeHillberg · 2023-07-25T19:38:53Z

specs/ScreenCaptureRequested.md

+                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/") 


Is this a safe comparison? The trailing "/" is guaranteed?

Please reuse other sample code that does URI comparison. Also compare just hostname.

MikeHillberg · 2023-07-25T19:42:38Z

specs/ScreenCaptureRequested.md

+
+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. 


Also, should we more align the name with getDisplayMedia?

MikeHillberg · 2023-07-25T19:43:25Z

specs/ScreenCaptureRequested.md

+  /// `CoreWebView2Frame` event handlers invoked first. The host may 
+  /// set this flag to `TRUE` within the `CoreWebView2Frame` event handlers 
+  /// to prevent the remaining `CoreWebView2` event handlers from being  
+  /// invoked. 


Also the remaining frame handlers from being invoked?

The text is currently correct. It will need to be updated in the future when we support grand+child frames.

MikeHillberg · 2023-07-25T19:44:56Z

specs/ScreenCaptureRequested.md

+  /// 
+  /// If a deferral is taken on the event args, then you must synchronously 
+  /// set `Handled` to TRUE prior to taking your deferral to prevent the 
+  /// `CoreWebView2`s event handlers from being invoked. 


Do we have this pattern anywhere else? Generally when you take a deferral, you can set results later.

Leave this as is. We do have this pattern elsewhere in WebView2.

Dave: Include this in how event bubbling should work

MikeHillberg · 2023-07-25T19:45:27Z

specs/ScreenCaptureRequested.md

+  [propput] HRESULT Handled([in] BOOL handled); 
+  /// The host may set this flag to cancel the screen capture. If canceled, 
+  /// the screen capture UI is not displayed regardless of the 
+  /// `Handled` property. 


(Although if you don't set Handled, someone downstream might clear your Cancel)

Yes. Please add this to the documentation.

david-risney

Please update per feedback, thanks!

david-risney · 2023-07-25T20:08:06Z

specs/ScreenCaptureRequested.md

+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 


Next time we'll separate out back story and future plans from rest of background.

david-risney · 2023-07-25T20:15:08Z

specs/ScreenCaptureRequested.md

+
+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. 


Let's rename this to align with the other ..Starting events

ScreenCaptureStarting

david-risney · 2023-07-25T20:19:00Z

specs/ScreenCaptureRequested.md

+`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


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

david-risney · 2023-07-25T20:21:18Z

specs/ScreenCaptureRequested.md

+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
+invoked.


Leave as is: match existing xaml / dom patterns of bubbling

david-risney · 2023-07-25T20:27:33Z

specs/ScreenCaptureRequested.md

+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
+invoked.


It does nothing in this case. We should explicitly document that is doing nothing in that case.

Dave: Follow up with how we want Handled to behave in this sort of case in the future.

david-risney · 2023-07-25T20:36:12Z

specs/ScreenCaptureRequested.md

+webView.CoreWebView2.ScreenCaptureRequested += (sender, screenCaptureArgs) =>
+{   
+    // Get Frame Info
+    wil::com_ptr<ICoreWebView2FrameInfo> frameInfo;


Yes please fix.

david-risney · 2023-07-25T20:36:52Z

specs/ScreenCaptureRequested.md

+```c#
+private WebView2 webView;
+
+m_webview.CoreWebView2.FrameCreated += (sender, frameCreatedArgs) =>


similar issue to above with member / local naming and logic. Please fix similar to above.

david-risney · 2023-07-25T20:40:10Z

specs/ScreenCaptureRequested.md

+  /// `CoreWebView2Frame` event handlers invoked first. The host may 
+  /// set this flag to `TRUE` within the `CoreWebView2Frame` event handlers 
+  /// to prevent the remaining `CoreWebView2` event handlers from being  
+  /// invoked. 


The text is currently correct. It will need to be updated in the future when we support grand+child frames.

david-risney · 2023-07-25T20:42:52Z

specs/ScreenCaptureRequested.md

+  /// 
+  /// If a deferral is taken on the event args, then you must synchronously 
+  /// set `Handled` to TRUE prior to taking your deferral to prevent the 
+  /// `CoreWebView2`s event handlers from being invoked. 


Leave this as is. We do have this pattern elsewhere in WebView2.

Dave: Include this in how event bubbling should work

david-risney · 2023-07-25T20:43:43Z

specs/ScreenCaptureRequested.md

+  [propput] HRESULT Handled([in] BOOL handled); 
+  /// The host may set this flag to cancel the screen capture. If canceled, 
+  /// the screen capture UI is not displayed regardless of the 
+  /// `Handled` property. 


Yes. Please add this to the documentation.

…ureStarting

srwei and others added 5 commits June 21, 2023 15:34

ScreenCaptureRequested draft

8f9fb8c

addressing comments

3b4a194

addressing comments on doc

99ee6b4

changed FrameInfo event arg to OriginalSourceFrameInfo for clarity, a…

52b4a00

…nd updated some documentation

Merge pull request #3587 from MicrosoftEdge/ScreenCaptureRequested-draft

b0fce82

ScreenCaptureRequested draft

srwei added the API Proposal Review WebView2 API Proposal for review. label Jul 20, 2023

srwei requested a review from david-risney July 20, 2023 23:55

mikeduglas reviewed Jul 21, 2023

View reviewed changes

oldnewthing reviewed Jul 21, 2023

View reviewed changes

MikeHillberg reviewed Jul 24, 2023

View reviewed changes

kmahone reviewed Jul 25, 2023

View reviewed changes

vickiez reviewed Jul 25, 2023

View reviewed changes

kmahone reviewed Jul 25, 2023

View reviewed changes

MikeHillberg reviewed Jul 25, 2023

View reviewed changes

david-risney requested changes Jul 25, 2023

View reviewed changes

updated from comments in API review board, changed name to ScreenCapt…

4ec2410

…ureStarting

david-risney added the review completed WebView2 API Proposal that's been reviewed and now needs final update and push label Aug 10, 2023

	if (frameSource = "https://developer.microsoft.com/en-us/microsoft-edge/webview2/")
	if (frameSource == "https://developer.microsoft.com/en-us/microsoft-edge/webview2/")

Screen Capture Requested #3655

Are you sure you want to change the base?

Screen Capture Requested #3655

Conversation

srwei commented Jul 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-risney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment