235026 – Enforce focus check for getUserMedia

RESOLVED FIXED 235026

Enforce focus check for getUserMedia

https://bugs.webkit.org/show_bug.cgi?id=235026

Summary Enforce focus check for getUserMedia

youenn fablet

Reported 2022-01-10 04:18:50 PST

As per spec, we should wait for focus before starting the getUserMedia algorithm

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
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2022-01-10 06:06:13 PST

Created attachment 448740 [details] Patch

youenn fablet

Comment 2 2022-01-10 07:09:23 PST

Created attachment 448747 [details] Patch

Eric Carlson

Comment 3 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

youenn fablet

Comment 4 2022-01-10 09:04:55 PST

Created attachment 448757 [details] Patch

youenn fablet

Comment 5 2022-01-11 10:14:06 PST

Created attachment 448853 [details] Patch

youenn fablet

Comment 6 2022-01-12 01:33:45 PST

Created attachment 448915 [details] Patch

EWS Watchlist

Comment 7 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

youenn fablet

Comment 8 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.

youenn fablet

Comment 9 2022-01-12 04:00:49 PST

Created attachment 448929 [details] Patch

youenn fablet

Comment 10 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.

youenn fablet

Comment 11 2022-01-12 06:40:56 PST

Created attachment 448940 [details] Patch

youenn fablet

Comment 12 2022-01-13 00:14:56 PST

Created attachment 449028 [details] Patch

Carlos Garcia Campos

Comment 13 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.

Carlos Garcia Campos

Comment 14 2022-01-13 06:23:25 PST

Created attachment 449055 [details] Patch

youenn fablet

Comment 15 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.

youenn fablet

Comment 16 2022-01-14 03:41:54 PST

Created attachment 449157 [details] Patch

youenn fablet

Comment 17 2022-01-14 05:06:10 PST

Created attachment 449165 [details] Patch

EWS

Comment 18 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].

Radar WebKit Bug Importer

Comment 19 2022-01-16 22:46:19 PST

<rdar://problem/87666800>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebRTC

Assignee

youenn fablet

Reported

2022-01-10 04:18 PST

Modified

2022-01-16 22:46 PST History

CC List

18 users Show

URL

Keywords InRadar

Depends on

Blocks