Bug 235026 - Enforce focus check for getUserMedia
Summary: Enforce focus check for getUserMedia
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-10 04:18 PST by youenn fablet
Modified: 2022-01-16 22:46 PST (History)
18 users (show)

See Also:


Attachments
Patch (18.29 KB, patch)
2022-01-10 06:06 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (18.13 KB, patch)
2022-01-10 07:09 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (19.98 KB, patch)
2022-01-10 09:04 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (51.66 KB, patch)
2022-01-11 10:14 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (57.47 KB, patch)
2022-01-12 01:33 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (57.44 KB, patch)
2022-01-12 04:00 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (58.21 KB, patch)
2022-01-12 06:40 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (58.23 KB, patch)
2022-01-13 00:14 PST, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (57.75 KB, patch)
2022-01-13 06:23 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (67.17 KB, patch)
2022-01-14 03:41 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (59.40 KB, patch)
2022-01-14 05:06 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2022-01-10 04:18:50 PST
As per spec, we should wait for focus before starting the getUserMedia algorithm
Comment 1 youenn fablet 2022-01-10 06:06:13 PST
Created attachment 448740 [details]
Patch
Comment 2 youenn fablet 2022-01-10 07:09:23 PST
Created attachment 448747 [details]
Patch
Comment 3 Eric Carlson 2022-01-10 09:01:47 PST
Comment on attachment 448747 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=448747&action=review

> Source/WebCore/ChangeLog:10
> +        The spec currently defines that we should wait for the current document to have focus but this is about to be changed and is not aligned with other browser implementations.

Nit: this line should be wrapped to make it easier to read
Comment 4 youenn fablet 2022-01-10 09:04:55 PST
Created attachment 448757 [details]
Patch
Comment 5 youenn fablet 2022-01-11 10:14:06 PST
Created attachment 448853 [details]
Patch
Comment 6 youenn fablet 2022-01-12 01:33:45 PST
Created attachment 448915 [details]
Patch
Comment 7 EWS Watchlist 2022-01-12 01:34:46 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 8 youenn fablet 2022-01-12 01:35:28 PST
@pnormand, can you look at GTK API test failures?
I tried to fix some of them but one should remain as we are now checking that the page is focused when getDisplayMedia is called, which does not seem to be the case when running the API test in GTK.
You can also probably fix the other GTK tests in the same way without introducing the private API as well.
Comment 9 youenn fablet 2022-01-12 04:00:49 PST
Created attachment 448929 [details]
Patch
Comment 10 youenn fablet 2022-01-12 05:07:15 PST
/webkit/WebKitWebView/display-usermedia-permission-request remains broken given the page does not have focus when calling getDisplayMedia.
Comment 11 youenn fablet 2022-01-12 06:40:56 PST
Created attachment 448940 [details]
Patch
Comment 12 youenn fablet 2022-01-13 00:14:56 PST
Created attachment 449028 [details]
Patch
Comment 13 Carlos Garcia Campos 2022-01-13 06:08:52 PST
(In reply to youenn fablet from comment #8)
> @pnormand, can you look at GTK API test failures?
> I tried to fix some of them but one should remain as we are now checking
> that the page is focused when getDisplayMedia is called, which does not seem
> to be the case when running the API test in GTK.
> You can also probably fix the other GTK tests in the same way without
> introducing the private API as well.

GTK failure is tricky. The view is always focused because we are synthesizing a click event that grab the focus in the view, but under Xvfb gtk_widget_grab_focus does nothing, because there's no toplevel focus support. So, we need to pretend the view is always focused and under the active window when running under Xvfb. I'll submit an updated patch.
Comment 14 Carlos Garcia Campos 2022-01-13 06:23:25 PST
Created attachment 449055 [details]
Patch
Comment 15 youenn fablet 2022-01-14 02:16:38 PST
Doing some more testing, it does not seem like document focus is the right approach.
If I call getUserMedia, the prompt will get the focus.
If shortly after calling getUserMedia, I call getDisplayMedia, the call will now fail while it would have before this patch.
Comment 16 youenn fablet 2022-01-14 03:41:54 PST
Created attachment 449157 [details]
Patch
Comment 17 youenn fablet 2022-01-14 05:06:10 PST
Created attachment 449165 [details]
Patch
Comment 18 EWS 2022-01-16 22:45:52 PST
Committed r288087 (246101@main): <https://commits.webkit.org/246101@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449165 [details].
Comment 19 Radar WebKit Bug Importer 2022-01-16 22:46:19 PST
<rdar://problem/87666800>