Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(564)

Issue 2833503002: OOPIF: Enable TabAndMouseFocusNavigation. (Closed)

Created:
1 year ago by avallee
Modified:
1 year ago
Reviewers:
Nico, alexmos
CC:
chromium-reviews, blink-reviews, Charlie Reis, dcheng, site-isolation-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

OOPIF: Enable TabAndMouseFocusNavigation. Reland https://codereview.chromium.org/2796533002 with synchronization with child frame navigation. When advancing focus into an iframe, allow the frame to focus itself after deciding which element will be focused. This prevents sending a focus event to the previous element in that frame only to blur it immediately. Non-oopifs simply fire a focus event to the new element, the previously focused element in that frame received a blur event when another frame was focused. The test ensures a consistent state when mixing tab and clicking navigation. In specific cases, focus would early out when the element to advance to was previously focused, and never cleared when the mouse is clicked in another frame. BUG=702330 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2833503002 Cr-Commit-Position: refs/heads/master@{#465692} Committed: https://chromium.googlesource.com/chromium/src/+/106f138fb306bb2283ce8adc22d1acd5eb31b891

Patch Set 1 #

Total comments: 4

Patch Set 2 : Fix comment by thakis. #

Patch Set 3 : Fix comment by alexmos. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -20 lines) Patch
M chrome/browser/site_per_process_interactive_browsertest.cc View 4 chunks +29 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/core/page/FocusController.cpp View 1 2 3 chunks +18 lines, -7 lines 0 comments Download

Messages

Total messages: 24 (16 generated)
avallee
Reland, added content::WaitForChildFrameSurfaceReady which ensures that we don't send click event meant for the child ...
1 year ago (2017-04-19 16:34:41 UTC) #2
avallee
thestig: dcheng seems OOO, mind reviewing on this reland cl, blink hasn't changed since https://codereview.chromium.org/2796533002.
1 year ago (2017-04-19 16:43:53 UTC) #6
avallee
thestig: sorry, copied the wrong name from git cl owners. thakis: please review the blink ...
1 year ago (2017-04-19 16:48:30 UTC) #8
Nico
pro tip for relanding CLs: Upload the original CL as patch set 1, then the ...
1 year ago (2017-04-19 16:53:16 UTC) #9
alexmos
LGTM to reland https://codereview.chromium.org/2833503002/diff/1/third_party/WebKit/Source/core/page/FocusController.cpp File third_party/WebKit/Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/2833503002/diff/1/third_party/WebKit/Source/core/page/FocusController.cpp#newcode1029 third_party/WebKit/Source/core/page/FocusController.cpp:1029: // ClearFocusedElement() fires events that might ...
1 year ago (2017-04-19 16:57:33 UTC) #12
avallee
Done. Thanks for the protip thakis. https://codereview.chromium.org/2833503002/diff/1/third_party/WebKit/Source/core/page/FocusController.cpp File third_party/WebKit/Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/2833503002/diff/1/third_party/WebKit/Source/core/page/FocusController.cpp#newcode1005 third_party/WebKit/Source/core/page/FocusController.cpp:1005: // Focus wrapped ...
1 year ago (2017-04-19 16:58:51 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2833503002/30001
1 year ago (2017-04-19 19:03:44 UTC) #20
commit-bot: I haz the power
1 year ago (2017-04-19 19:09:02 UTC) #23
Message was sent while issue was closed.
Committed patchset #3 (id:30001) as
https://chromium.googlesource.com/chromium/src/+/106f138fb306bb2283ce8adc22d1...

Powered by Google App Engine
This is Rietveld 408576698