ScreenCaptureRequested draft #3587

srwei · 2023-06-21T22:35:45Z

No description provided.

david-risney · 2023-06-21T23:17:02Z

specs/ScreenCaptureRequested

@@ -0,0 +1,394 @@
+# Background
+
+JavaScript's Screen Capture API `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. 


Please limit line lengths to 80 or 100 characters or less to make it easier to read in the PR.

david-risney · 2023-06-26T18:12:00Z

specs/ScreenCaptureRequested

+[uuid(a1d309ee-c03f-11eb-8529-0242ac130003), object, pointer_default(unique)] 
+interface ICoreWebView2ScreenCaptureRequestedEventArgs : IUnknown { 
+  /// The associated frame information that requests the screen capture  
+  /// permission. This can be used to grab the frame source, name, frameId,  


This also includes URI of the frame? I expect knowing the URI will be important to deciding to allow or disallow screen capture

Yeah, FrameInfo.Source is the URI of the frame, which also is valid for the top level document.

david-risney · 2023-06-26T18:18:07Z

specs/ScreenCaptureRequested

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


Please also document what happens in script. I guess that getDisplayMedia will asynchronously fail with the NotAllowedError error?

david-risney · 2023-06-26T18:18:44Z

specs/ScreenCaptureRequested

+[uuid(12885cda-9caa-4793-9c38-f15827dbab1f), object, pointer_default(unique)] 
+interface ICoreWebView2Frame3 : IUnknown { 
+  /// Add an event handler for the `ScreenCaptureRequested` event. 
+  /// `ScreenCaptureRequested is raised when content in an iframe any of its 


Suggested change

/// `ScreenCaptureRequested is raised when content in an iframe any of its

/// `ScreenCaptureRequested is raised when content in an iframe or any of its

david-risney · 2023-06-26T18:21:32Z

specs/ScreenCaptureRequested

+  /// be raised on the `CoreWebView2`, and its event handlers will not be   
+  /// invoked. 
+  /// 
+  /// In the case of nested iframes, the 'ScreenCaptureRequested' event will 


You already say above that descendant iframes will raise the event. In the future we may support nested iframes. I don't think we need this sentence.

david-risney · 2023-06-26T18:21:55Z

specs/ScreenCaptureRequested

+  /// be raised from the top level iframe. 
+  /// 
+  /// If a deferral is not taken on the event args, the subsequent scripts  
+  /// re blocked until the event handler returns.  If a deferral is taken,  


Suggested change

/// re blocked until the event handler returns. If a deferral is taken,

/// are blocked until the event handler returns. If a deferral is taken,

david-risney · 2023-06-26T18:24:13Z

specs/ScreenCaptureRequested

+  /// In the case of nested iframes, the 'ScreenCaptureRequested' event will 
+  /// be raised from the top level iframe. 
+  /// 
+  /// If a deferral is not taken on the event args, the subsequent scripts  


Is this right? Subsequent scripts aren't blocked are they? I would have thought all that is blocked is async completion of getDisplayMedia method?

david-risney · 2023-06-26T18:27:20Z

specs/ScreenCaptureRequested

+                wil::com_ptr<ICoreWebView2StagingFrameInfo> frameInfoStaging;
+                CHECK_FAILURE(frameInfo->QueryInterface(IID_PPV_ARGS(&frameInfoStaging)));
+                UINT32 frameId = 0;
+                frameInfoStaging->get_FrameId(&frameId);


You don't do anything with the values you get. You don't need to get every property value but obtain some of them and show a real use of the properties to decide to cancel or not.

david-risney · 2023-06-26T18:30:10Z

specs/ScreenCaptureRequested

+        &m_FrameCreatedToken));
+}
+
+static void PutHandled(ICoreWebView2ScreenCaptureRequestedEventArgs* args)


It doesn't seem like you need a separate static function here. You check if args != null but when you call PutHandled just above it you do args->put_Cancel so args should be non null.

david-risney · 2023-06-26T18:31:58Z

specs/ScreenCaptureRequested

+    frameId = frameInfo.FrameId;
+
+    // If the host app wants to cancel the request from a specific frame
+    if ( frameId == 1 || frameSource == "https://developer.microsoft.com/en-us/microsoft-edge/webview2/"


Is frameId of 1 special? Or are you hardcoding as an example for this code sample? If so, a better and more realistic example would be just the URI check and not the frame name or frame ID and a good enough sample for the other cases of checking frame ID or frame name.

@david-risney Yes, it was just hard coded. That makes sense to just use the URI to cancel. What do you mean by "a good enough sample for the other cases of checking frame ID or frame name?"

I mean there may be cases where someone wants to check frame ID. If our sample only shows checking the frame URI it should be easy enough for the reader to figure out how to change that to instead check the frame ID. And if we check only the frame URI it should be easier to make a more realistic sample.

david-risney · 2023-07-17T21:31:36Z

specs/ScreenCaptureRequested.md

+    {
+        // ...
+        // ICoreWebView2Frame3 members
+        [doc_string("ScreenCaptureRequested is raised when content in an iframe or any of its descendant iframes requests permission to use screen capture.\nIf a deferral is not taken on the event args, the subsequent scripts are blocked until the event handler returns. If a deferral is taken, the scripts are blocked until the deferral is completed.")]


This line is longer than 80/100 chars, but also the only instance of documentation in the MIDL3. I'd recommend removing this and only having the docs in the COM IDL above to avoid differences between the two and so on.

david-risney · 2023-07-17T21:33:30Z

specs/ScreenCaptureRequested

@@ -0,0 +1,394 @@
+# Background


Why are there two documents? One is ScreenCaptureRequested and the other is ScreenCaptureRequested.md

Accidently created the first one without the markdown extension. Removing the old one. Thanks for catching that!

david-risney · 2023-07-17T21:35:43Z

specs/ScreenCaptureRequested.md

+
+            // Let CoreWebView2 handler know the event is already handled
+
+            // In the case of an iframe requesting permission to use Screen Capture, the default 


This first part of the comment about the event going from frame to parent is good and should also appear in the Win32 sample code.

david-risney · 2023-07-17T21:45:33Z

specs/ScreenCaptureRequested.md

+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. 
+
+To maintain backwards compatibility, by default we plan to raise


We aren't doing this for compat. This is a new event. We're adding it on both frame and webview2 for convenience of end dev.

david-risney

Looking good. Two changes and then please complete the PR, open a new PR to main, let me know, and I will schedule a review. Thanks

david-risney · 2023-07-19T18:09:07Z

specs/ScreenCaptureRequested.md

@@ -0,0 +1,357 @@
+# Background
+
+JavaScript's Screen Capture API `GetDisplayMedia` allows developers to get a video stream of a 


Suggested change

JavaScript's Screen Capture API `GetDisplayMedia` allows developers to get a video stream of a

The HTML DOM's Screen Capture API `navigator.mediaDevices.getDisplayMedia` allows developers to get a video stream of a

david-risney · 2023-07-20T20:59:59Z

specs/ScreenCaptureRequested.md

+  /// The associated frame information that requests the screen capture  
+  /// permission. This can be used to grab the frame source, name, frameId,  
+  /// and parent frame information. 
+  [propget] HRESULT FrameInfo([out, retval] ICoreWebView2FrameInfo**  


I'd like to use this same pattern in other events in the future. It will be good to put a prefix on this name to make it clear what this is the frame info for. Without a prefix it may be confused for the frame info of the frame on which the event was registered rather than the frame info of the original source of the event.

XAML calls this OriginalSource
HTML DOM events uses event.target for the original source of the event as opposed to event.currentTarget to refer to the current DOM node for bubbling.

I find the HTML DOM events one more confusing. How about OriginalSourceFrameInfo?

Good point. I think OriginalSourceFrameInfo makes sense.

…nd updated some documentation

ScreenCaptureRequested draft

8f9fb8c

srwei requested a review from david-risney June 21, 2023 22:35

srwei added the API Proposal Review WebView2 API Proposal for review. label Jun 21, 2023

david-risney requested a review from aluhrs13 June 26, 2023 17:55

david-risney requested changes Jun 26, 2023

View reviewed changes

addressing comments

3b4a194

srwei requested a review from david-risney July 5, 2023 23:49

david-risney requested changes Jul 17, 2023

View reviewed changes

addressing comments on doc

99ee6b4

srwei requested a review from david-risney July 19, 2023 18:00

david-risney requested changes Jul 20, 2023

View reviewed changes

changed FrameInfo event arg to OriginalSourceFrameInfo for clarity, a…

52b4a00

…nd updated some documentation

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

srwei merged commit b0fce82 into ScreenCaptureRequested Jul 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ScreenCaptureRequested draft #3587

ScreenCaptureRequested draft #3587

srwei commented Jun 21, 2023

david-risney Jun 21, 2023

david-risney Jun 26, 2023

srwei Jun 26, 2023

david-risney Jun 26, 2023

david-risney Jun 26, 2023

david-risney Jun 26, 2023

david-risney Jun 26, 2023

david-risney Jun 26, 2023

david-risney Jun 26, 2023

david-risney Jun 26, 2023

david-risney Jun 26, 2023

srwei Jul 5, 2023

david-risney Jul 5, 2023

david-risney Jul 17, 2023

david-risney Jul 17, 2023

srwei Jul 19, 2023

david-risney Jul 17, 2023

david-risney Jul 17, 2023

david-risney left a comment

david-risney Jul 19, 2023

david-risney Jul 20, 2023

srwei Jul 20, 2023

		@@ -0,0 +1,394 @@
		# Background

		JavaScript's Screen Capture API `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.

	/// `ScreenCaptureRequested is raised when content in an iframe any of its
	/// `ScreenCaptureRequested is raised when content in an iframe or any of its

	/// re blocked until the event handler returns. If a deferral is taken,
	/// are blocked until the event handler returns. If a deferral is taken,


		// Let CoreWebView2 handler know the event is already handled

		// In the case of an iframe requesting permission to use Screen Capture, the default

		@@ -0,0 +1,357 @@
		# Background

		JavaScript's Screen Capture API `GetDisplayMedia` allows developers to get a video stream of a

	JavaScript's Screen Capture API `GetDisplayMedia` allows developers to get a video stream of a
	The HTML DOM's Screen Capture API `navigator.mediaDevices.getDisplayMedia` allows developers to get a video stream of a

ScreenCaptureRequested draft #3587

ScreenCaptureRequested draft #3587

Conversation

srwei commented Jun 21, 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

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