New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 702330 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

OOPIF: Tabbing into a subframe can focus the wrong element

Project Member Reported by aval...@chromium.org, Mar 16 2017

Issue description

As described in 674219 c6, tabbing from a parent frame into a subframe will focus the frame and then focus the element after the previously focus element in the subframe.

When an tabbing into a subframe, the focus event should be sent to the previously focused element, not the one before or after it.

Original comment:
=====
1. Go to http://csreis.github.io/tests/cross-site-iframe-simple.html with
--site-per-process.
2. In devtools, 
document.querySelector("h1").appendChild(document.createElement("input"))
3. Click first <input> in iframe
4. Click the main frame's <input> created in step 2.
5. Press tab

Focus should switch to the first <input> in the iframe, but instead it
shifts to the *second* <input> there.
 
Summary: OOPIF: Tabbing into a subframe can focus the wrong element (was: OOPIF: Tabbing into a subframe can focus )
Another strange case, without oopif.

If I focus an element in an inner frame and then click outside the frame, then innerDocument.activeElement becomes body. If I tab back around the page and get back to the iframe, the first focusable element is the one that becomes focused.

The slightly crazy thing is that if I call iframe.focus() the innerDocument.activeElement is still body, but now if I tab, the focused element is now the oldFocusedElement + 1.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28b32ffa898387ee0decdd02a17fd0e45f095772

commit 28b32ffa898387ee0decdd02a17fd0e45f095772
Author: avallee <avallee@chromium.org>
Date: Tue Mar 28 03:30:51 2017

OOPIF: Add test to for tab and mouse focus changes.

Test ensures that focus can transition to parent or sibling frames when
the target element to be focused was clicked just before the frame was
focused.

Test currently fails due to  http://crbug.com/702330 .

BUG= 702330 

Review-Url: https://codereview.chromium.org/2771213003
Cr-Commit-Position: refs/heads/master@{#459998}

[modify] https://crrev.com/28b32ffa898387ee0decdd02a17fd0e45f095772/chrome/browser/site_per_process_interactive_browsertest.cc

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a539b43629daa2a49d5603fea5ade7d48d31dd2

commit 6a539b43629daa2a49d5603fea5ade7d48d31dd2
Author: avallee <avallee@chromium.org>
Date: Fri Apr 07 21:16:06 2017

OOPIF: Enable TabAndMouseFocusNavigation.

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/2796533002
Cr-Commit-Position: refs/heads/master@{#463006}

[modify] https://crrev.com/6a539b43629daa2a49d5603fea5ade7d48d31dd2/chrome/browser/site_per_process_interactive_browsertest.cc
[modify] https://crrev.com/6a539b43629daa2a49d5603fea5ade7d48d31dd2/third_party/WebKit/Source/core/page/FocusController.cpp

Comment 6 by creis@chromium.org, Apr 8 2017

Sorry, having to revert because the test is flaky:
https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/54519

SitePerProcessInteractiveBrowserTest.TabAndMouseFocusNavigation (run #1):
[ RUN      ] SitePerProcessInteractiveBrowserTest.TabAndMouseFocusNavigation
Xlib:  extension "RANDR" missing on display ":99".
[26835:26835:0407/164748.326788:WARNING:audio_manager.cc(295)] Multiple instances of AudioManager detected
[26835:26835:0407/164748.326809:WARNING:audio_manager.cc(254)] Multiple instances of AudioManager detected
[26835:26835:0407/164748.345639:WARNING:password_store_factory.cc(249)] Using basic (unencrypted) store for password storage. See https://chromium.googlesource.com/chromium/src/+/master/docs/linux_password_storage.md for more information about password storage options.
[26835:26835:0407/164748.505911:WARNING:render_frame_host_impl.cc(2347)] OnDidStopLoading was called twice.
[26835:26835:0407/164748.506546:WARNING:render_frame_host_impl.cc(2347)] OnDidStopLoading was called twice.
[26835:26894:0407/164748.511304:WARNING:embedded_test_server.cc(219)] Request not handled. Returning 404: /favicon.ico
BrowserTestBase received signal: Terminated. Backtrace:
#0 0x0000023bef17 base::debug::StackTrace::StackTrace()
#1 0x000001d7d6f6 content::(anonymous namespace)::DumpStackTraceSignalHandler()
#2 0x7f4221fdccb0 <unknown>
#3 0x7f4222092fdd __poll
#4 0x7f42278fefe4 <unknown>
#5 0x7f42278ff0ec g_main_context_iteration
#6 0x0000023d8e46 base::MessagePumpGlib::Run()
#7 0x0000023d71c7 base::MessageLoop::RunHandler()
#8 0x0000023f51ae base::RunLoop::Run()
#9 0x000001d85b11 content::MessageLoopRunner::Run()
#10 0x000001d7efa2 content::DOMMessageQueue::WaitForMessage()
#11 0x00000053cbea SitePerProcessInteractiveBrowserTest_TabAndMouseFocusNavigation_Test::RunTestOnMainThread()::$_4::operator()()
#12 0x00000053a48d SitePerProcessInteractiveBrowserTest_TabAndMouseFocusNavigation_Test::RunTestOnMainThread()
#13 0x000001a0075f InProcessBrowserTest::RunTestOnMainThreadLoop()
#14 0x000001d7d458 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#15 0x000001c847f4 ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#16 0x000001c8389b ChromeBrowserMainParts::PreMainMessageLoopRun()
#17 0x000000b119bf content::BrowserMainLoop::PreMainMessageLoopRun()
#18 0x000000dd7f57 content::StartupTaskRunner::RunAllTasksNow()
#19 0x000000b0fc75 content::BrowserMainLoop::CreateStartupTasks()
#20 0x000000b142a4 content::BrowserMainRunnerImpl::Initialize()
#21 0x000000b0d3d2 content::BrowserMain()
#22 0x0000019f2d3c content::ContentMainRunnerImpl::Run()
#23 0x000002df4ecf service_manager::Main()
#24 0x0000019f1db2 content::ContentMain()
#25 0x000001d7cd99 content::BrowserTestBase::SetUp()
#26 0x0000019ff600 InProcessBrowserTest::SetUp()
#27 0x0000022e1c3e testing::Test::Run()
#28 0x0000022e27b0 testing::TestInfo::Run()
#29 0x0000022e2cd7 testing::TestCase::Run()
#30 0x0000022e9d17 testing::internal::UnitTestImpl::RunAllTests()
#31 0x0000022e9997 testing::UnitTest::Run()
#32 0x0000023ab3a1 base::TestSuite::Run()
#33 0x0000005e2243 InteractiveUITestSuiteRunner::RunTestSuite()
#34 0x000001d8236a content::LaunchTests()
#35 0x0000005e21c8 main
#36 0x7f4221fc7f45 __libc_start_main
#37 0x000000486f50 <unknown>
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b7e3fac6988ecb6dd1d2465aee41e473724c9d2

commit 3b7e3fac6988ecb6dd1d2465aee41e473724c9d2
Author: creis <creis@chromium.org>
Date: Sat Apr 08 00:25:27 2017

Revert of OOPIF: Enable TabAndMouseFocusNavigation. (patchset #8 id:140001 of https://codereview.chromium.org/2796533002/ )

Reason for revert:
Test is flaky on Linux Tests bot:
https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/54519

Original issue's description:
> OOPIF: Enable TabAndMouseFocusNavigation.
>
> 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/2796533002
> Cr-Commit-Position: refs/heads/master@{#463006}
> Committed: https://chromium.googlesource.com/chromium/src/+/6a539b43629daa2a49d5603fea5ade7d48d31dd2

TBR=alexmos@chromium.org,dcheng@chromium.org,avallee@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 702330 

Review-Url: https://codereview.chromium.org/2804303004
Cr-Commit-Position: refs/heads/master@{#463071}

[modify] https://crrev.com/3b7e3fac6988ecb6dd1d2465aee41e473724c9d2/chrome/browser/site_per_process_interactive_browsertest.cc
[modify] https://crrev.com/3b7e3fac6988ecb6dd1d2465aee41e473724c9d2/third_party/WebKit/Source/core/page/FocusController.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/106f138fb306bb2283ce8adc22d1acd5eb31b891

commit 106f138fb306bb2283ce8adc22d1acd5eb31b891
Author: avallee <avallee@chromium.org>
Date: Wed Apr 19 19:08:46 2017

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}

[modify] https://crrev.com/106f138fb306bb2283ce8adc22d1acd5eb31b891/chrome/browser/site_per_process_interactive_browsertest.cc
[modify] https://crrev.com/106f138fb306bb2283ce8adc22d1acd5eb31b891/third_party/WebKit/Source/core/page/FocusController.cpp

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1668be8ea0b0189b21966b96424ddf3308b57c7f

commit 1668be8ea0b0189b21966b96424ddf3308b57c7f
Author: avallee <avallee@chromium.org>
Date: Fri Apr 21 16:33:49 2017

Disable OOPIF test TabAndMouseFocusNavigation on windows.

BUG= 702330 ,  713977 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2828283004
Cr-Commit-Position: refs/heads/master@{#466361}

[modify] https://crrev.com/1668be8ea0b0189b21966b96424ddf3308b57c7f/chrome/browser/site_per_process_interactive_browsertest.cc

Sign in to add a comment