Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(592)

Issue 2833503002: OOPIF: Enable TabAndMouseFocusNavigation. (Closed)

Created:
3 years, 8 months ago by avallee
Modified:
3 years, 8 months 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 ...
3 years, 8 months 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.
3 years, 8 months 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 ...
3 years, 8 months 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 ...
3 years, 8 months 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 ...
3 years, 8 months 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 ...
3 years, 8 months 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
3 years, 8 months ago (2017-04-19 19:03:44 UTC) #20
commit-bot: I haz the power
3 years, 8 months 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