Bug 185617

Summary:

Keyboard focus should exit fullscreen.

Product:

WebKit

Reporter:

Jeremy Jones <jeremyj-wk>

Component:

WebKit2

Assignee:

Jeremy Jones <jeremyj-wk>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, bfulgham, cdumez, cmarcelo, commit-queue, dbates, eric.carlson, esprehn+autocc, ews-watchlist, jer.noble, jonlee, kangil.han, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Archive of layout-test-results from ews122 for ios-simulator-wk2	none
Archive of layout-test-results from ews206 for win-future	none
fix sim experiment	none
Patch	none

Jeremy Jones

Reported 2018-05-14 10:47:32 PDT

Keyboard focus should exit fullscreen.

Attachments
Patch (1.48 KB, patch) 2018-05-14 10:51 PDT, Jeremy Jones	no flags	Details Formatted Diff Diff
Patch (7.02 KB, patch) 2018-05-25 07:59 PDT, Jeremy Jones	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (deleted) 2018-05-25 09:48 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews206 for win-future (12.74 MB, application/zip) 2018-05-25 14:19 PDT, EWS Watchlist	no flags	Details
fix sim experiment (7.48 KB, patch) 2018-05-25 15:13 PDT, Jeremy Jones	no flags	Details Formatted Diff Diff
Patch (5.11 KB, patch) 2018-06-01 15:09 PDT, Jeremy Jones	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Jeremy Jones

Comment 1 2018-05-14 10:49:23 PDT

rdar://problem/34697938

Jeremy Jones

Comment 2 2018-05-14 10:51:38 PDT

Created attachment 340330 [details] Patch

Ryosuke Niwa

Comment 3 2018-05-21 15:19:24 PDT

Comment on attachment 340330 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340330&action=review > Source/WebKit/WebProcess/WebPage/WebPage.cpp:4913 > + node->document().webkitCancelFullScreen(); Surely, this can be tested? Also, do we really want to do this on both macOS and iOS?

Jeremy Jones

Comment 4 2018-05-25 07:10:48 PDT

(In reply to Ryosuke Niwa from comment #3) > Comment on attachment 340330 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=340330&action=review > > > Source/WebKit/WebProcess/WebPage/WebPage.cpp:4913 > > + node->document().webkitCancelFullScreen(); > > Surely, this can be tested? > Also, do we really want to do this on both macOS and iOS? Working on a test. We only want to do this on iOS. #if PLATFORM(IOS) + +#if ENABLE(FULLSCREEN_API) + node->document().webkitCancelFullScreen(); +#endif +

Jeremy Jones

Comment 5 2018-05-25 07:59:45 PDT

Created attachment 341281 [details] Patch

Eric Carlson

Comment 6 2018-05-25 09:40:43 PDT

Comment on attachment 341281 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341281&action=review > LayoutTests/fullscreen/full-screen-exit-on-keyboard-focus.html:50 > + setTimeout(function() { > + consoleWrite("Attempt to focus on editable content."); > + runOnMouseDown(function() {}); > + }, 500); I don't understand this - why run it after 500ms, and what does runOnMouseDown with an empty lambda do? > LayoutTests/fullscreen/full-screen-exit-on-keyboard-focus.html:77 > +<body Xonmousedown="doMouseDown()"> Xonmousedown?

EWS Watchlist

Comment 7 2018-05-25 09:48:32 PDT

Comment on attachment 341281 [details] Patch Attachment 341281 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/7800224 New failing tests: fullscreen/full-screen-exit-on-keyboard-focus.html

EWS Watchlist

Comment 8 2018-05-25 09:48:34 PDT

Created attachment 341295 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4

EWS Watchlist

Comment 9 2018-05-25 14:19:09 PDT

Comment on attachment 341281 [details] Patch Attachment 341281 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7802859 New failing tests: http/tests/security/canvas-remote-read-remote-video-localhost.html

EWS Watchlist

Comment 10 2018-05-25 14:19:20 PDT

Created attachment 341328 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Jeremy Jones

Comment 11 2018-05-25 15:13:00 PDT

Created attachment 341332 [details] fix sim experiment

Ryosuke Niwa

Comment 12 2018-05-29 17:41:10 PDT

Comment on attachment 341281 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341281&action=review > LayoutTests/fullscreen/full-screen-exit-on-keyboard-focus.html:47 > + setTimeout(function() { I think we should expose some boolean indicating the fullscreen transition had finished or not via internals, and then we can poll that value here instead of 500ms. I'm afraid the test will be flaky in some slow bots otherwise. Namely, set this Document flag true in Document::webkitDidEnterFullScreenForElement and clear it in Document::webkitWillEnterFullScreenForElement and Document::webkitWillExitFullScreenForElement >> LayoutTests/fullscreen/full-screen-exit-on-keyboard-focus.html:77 >> +<body Xonmousedown="doMouseDown()"> > > Xonmousedown? Plase remove this as we discussed in person.

Jeremy Jones

Comment 13 2018-06-01 15:09:02 PDT

Created attachment 341792 [details] Patch

WebKit Commit Bot

Comment 14 2018-06-01 16:23:03 PDT

Comment on attachment 341792 [details] Patch Clearing flags on attachment: 341792 Committed r232427: <https://trac.webkit.org/changeset/232427>

WebKit Commit Bot

Comment 15 2018-06-01 16:23:05 PDT

All reviewed patches have been landed. Closing bug.

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