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

Issue 719788 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Inert/<dialog> doesn't work across OOPIFs currently

Project Member Reported by aboxhall@chromium.org, May 9 2017

Issue description

What steps will reproduce the problem?

(1) With --site-per-process enabled, navigate to http://csreis.github.io/tests/cross-site-iframe-simple.html

(2) Using devtools, run the following JS in the IFRAME context:

document.head.innerHTML = '';
document.body.innerHTML = '<input id="text1"> <input id="text2">';
text1.focus();
document.body.addEventListener('focus', (e) => { console.log('got focus event on', e.target) }, true);

(3) Using devtools, run the following JS in the TOP context:

let dialog = document.body.appendChild(document.createElement('dialog'));
dialog.innerHTML = "This is a modal dialog. <input>";
dialog.showModal();

(4) Using devtools, run the following JS in the IFRAME context:

window.setTimeout(() => { console.log('about to focus', text2); text2.focus(); }, 5000);

(5) IMMEDIATELY move focus to the main browser window (via Alt+Tab or using the mouse to click on the window top bar avoiding clicking on any focusable items).


What is the expected result?

- The <input> in the modal dialog is shown as focus when the browser window is focused, and remains focused even after the timeout causes the log statement to be printed.
- No 'got focus event on' log statement should be printed.

(This can be verified by running these steps without --site-per-process enabled.)

What happens instead?

- After the timeout finishes, "got focus event on <input id=​"text2">​…​</input>​" is logged as a result of the focus() call.

 

Comment 1 by kenrb@chromium.org, May 9 2017

Cc: kenrb@chromium.org dcheng@chromium.org alex...@chromium.org
Thanks for filing this.

I am guessing we need to propagate an 'inert' flag to any RemoteFrame children when dialog.showModal() is called, which would be stored on RemoteFrameOwner in the OOPIF process.

Comment 2 Deleted

That sounds about right. Also note that inert will soon be able to be set via attribute: https://bugs.chromium.org/p/chromium/issues/detail?id=692360 - but we can figure out how to get that to trigger the inert flag then.
FWIW, this will also make the implementation of IsInert simpler, because right now it tries to walk all local frames, but other frames may not be up-to-date (e.g. UpdateDistribution(), layout, etc.). I think each frame should know whether or not the whole frame is inert or not, and then when setting inert or calling showModal, that should be propagated to all other frames.

Comment 5 by kenrb@chromium.org, May 15 2017

Cc: -kenrb@chromium.org
Owner: kenrb@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 23 2017

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

commit 04323789a189306e523b5982a9cd49dcebdc88f5
Author: kenrb <kenrb@chromium.org>
Date: Fri Jun 23 01:23:32 2017

Propagate inert state to OOPIFs when a modal dialog is active

When showModal is called on a <dialog> element, the rest of the
Document becomes inert which prevents it from receiving events or taking
focus. When the Document contains an out-of-process iframe, however, its
contents are not aware of the modal dialog in a remote ancestor.

This CL caches the current inert state on each LocalFrame, which is
changed when a modal dialog because active or inactive. The bit is
plumbed to remote frame children so that OOPIFs will respect inertness.

Also it prevents having to search up the entire frame tree when an
element checks whether it is inert.

BUG= 719788 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/browser/frame_host/cross_process_frame_connector.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/browser/frame_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/common/frame_messages.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/common/view_messages.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/renderer/render_frame_proxy.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/renderer/render_widget.cc
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/content/renderer/render_widget.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/LayoutTests/html/dialog/inert-focus-in-frames-expected.txt
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/LayoutTests/html/dialog/inert-focus-in-frames.html
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/LayoutTests/html/dialog/resources/inert-focus-in-frames-frame1.html
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/Frame.cpp
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/Frame.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/LocalFrame.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/RemoteFrame.cpp
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/RemoteFrame.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/RemoteFrameClient.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/RemoteFrameClientImpl.cpp
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/frame/RemoteFrameClientImpl.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/html/HTMLDialogElement.cpp
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/html/HTMLElement.cpp
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/core/loader/EmptyClients.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/Source/web/WebFrameWidgetImpl.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/public/web/WebFrameWidget.h
[modify] https://crrev.com/04323789a189306e523b5982a9cd49dcebdc88f5/third_party/WebKit/public/web/WebRemoteFrameClient.h

Comment 7 by kenrb@chromium.org, Jul 14 2017

Status: Fixed (was: Started)
Fixed a while ago, forgot to close this bug.

Sign in to add a comment