From 168b2347b31aa1a84fa624cb3ae634cb5955798f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 31 Mar 2021 01:51:46 +0000 Subject: [PATCH] Bug 1700871 - Only allow focus move for links / form submission iff actually handling user input. r=smaug The other navigation that allows focus moves (window.open) already goes through the popup blocker, so that one is fine. I think given how weird yet conservative other browsers are, this should be a good trade-off to avoid false positives. Differential Revision: https://phabricator.services.mozilla.com/D110196 --- docshell/base/nsDocShell.cpp | 2 +- dom/base/test/browser.ini | 1 + dom/base/test/browser_bug1303838.js | 18 +++----- dom/base/test/browser_bug1691214.js | 64 ++++++++++++++++++++++++----- dom/base/test/file_bug1303838.html | 35 ++++++++++------ dom/base/test/file_bug1700871.html | 18 ++++++++ dom/html/HTMLFormElement.cpp | 1 + 7 files changed, 102 insertions(+), 37 deletions(-) create mode 100644 dom/base/test/file_bug1700871.html diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 52e42d551c4e..5bae8dca1b3d 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -12847,6 +12847,7 @@ nsresult nsDocShell::OnLinkClick( aTriggeringPrincipal ? aTriggeringPrincipal : aContent->NodePrincipal()); loadState->SetPrincipalToInherit(aContent->NodePrincipal()); loadState->SetCsp(aCsp ? aCsp : aContent->GetCsp()); + loadState->SetAllowFocusMove(UserActivation::IsHandlingUserInput()); nsCOMPtr ev = new OnLinkClickEvent(this, aContent, loadState, noOpenerImplied, @@ -13011,7 +13012,6 @@ nsresult nsDocShell::OnLinkClickSync(nsIContent* aContent, aLoadState->SetTypeHint(NS_ConvertUTF16toUTF8(typeHint)); aLoadState->SetLoadType(loadType); aLoadState->SetSourceBrowsingContext(mBrowsingContext); - aLoadState->SetAllowFocusMove(true); aLoadState->SetHasValidUserGestureActivation( context && context->HasValidTransientUserGestureActivation()); diff --git a/dom/base/test/browser.ini b/dom/base/test/browser.ini index bc5b7e1aa05b..dc38c9edfc08 100644 --- a/dom/base/test/browser.ini +++ b/dom/base/test/browser.ini @@ -69,6 +69,7 @@ skip-if = os == 'win' && bits == 64 # Bug 1692963 support-files = file_bug1691214.html + file_bug1700871.html [browser_inputStream_structuredClone.js] [browser_multiple_popups.js] skip-if = (os == 'win' && !debug) || (os == "mac" && !debug) # Bug 1505235, Bug 1661132 (osx) diff --git a/dom/base/test/browser_bug1303838.js b/dom/base/test/browser_bug1303838.js index 357167394d32..69e67b33c03c 100644 --- a/dom/base/test/browser_bug1303838.js +++ b/dom/base/test/browser_bug1303838.js @@ -291,19 +291,13 @@ function clickLink( waitForLocationChange(targetsFrame, testBrowser, locationChangeNum) ); } + + info("BC children: " + browser.browsingContext.children.length); promises.push( - SpecialPowers.spawn( - browser, - [[isFrame, linkId]], - ([contentIsFrame, contentLinkId]) => { - let doc = content.document; - if (contentIsFrame) { - let frame = content.document.getElementById("frame"); - doc = frame.contentDocument; - } - info("Clicking " + contentLinkId); - doc.querySelector(contentLinkId).click(); - } + BrowserTestUtils.synthesizeMouseAtCenter( + linkId, + {}, + isFrame ? browser.browsingContext.children[0] : browser ) ); return Promise.all(promises); diff --git a/dom/base/test/browser_bug1691214.js b/dom/base/test/browser_bug1691214.js index 56cde6a3b6bb..488f4551b202 100644 --- a/dom/base/test/browser_bug1691214.js +++ b/dom/base/test/browser_bug1691214.js @@ -6,25 +6,28 @@ const BASE_URL = "http://mochi.test:8888/browser/dom/base/test/"; -add_task(async function() { +async function newFocusedWindow() { + let delayedStartupPromise = BrowserTestUtils.waitForNewWindow(); + let win = await BrowserTestUtils.domWindowOpenedAndLoaded(); + // New windows get focused after the first paint, see bug 1262946 + await BrowserTestUtils.waitForContentEvent( + win.gBrowser.selectedBrowser, + "MozAfterPaint" + ); + await delayedStartupPromise; + return win; +} + +add_task(async function bug1691214() { await BrowserTestUtils.withNewTab( BASE_URL + "file_bug1691214.html", async function(browser) { let win; { - let newWindow = BrowserTestUtils.domWindowOpenedAndLoaded(); - let delayedStartupPromise = BrowserTestUtils.waitForNewWindow(); - + let newWindow = newFocusedWindow(); await BrowserTestUtils.synthesizeMouseAtCenter("#link-1", {}, browser); win = await newWindow; - ok(win, "First navigation should've opened the new window"); - // New windows get focused after the first paint, see bug 1262946 - await BrowserTestUtils.waitForContentEvent( - win.gBrowser.selectedBrowser, - "MozAfterPaint" - ); - await delayedStartupPromise; is(Services.focus.focusedWindow, win, "New window should be focused"); } @@ -56,3 +59,42 @@ add_task(async function() { } ); }); + +// The tab has a form infinitely submitting to an iframe, and that shouldn't +// switch focus back. For that, we open a window from the tab, make sure it's +// focused, and then wait for three submissions (but since we need to listen to +// trusted events from chrome code, we use the iframe load event instead). +add_task(async function bug1700871() { + await BrowserTestUtils.withNewTab( + BASE_URL + "file_bug1700871.html", + async function(browser) { + let win; + + { + let newWindow = newFocusedWindow(); + await BrowserTestUtils.synthesizeMouseAtCenter("#link-1", {}, browser); + win = await newWindow; + is(Services.focus.focusedWindow, win, "New window should be focused"); + } + + info("waiting for three submit events and ensuring focus hasn't moved"); + + await SpecialPowers.spawn(browser, [], function() { + let pending = 3; + return new Promise(resolve => { + content.document + .querySelector("iframe") + .addEventListener("load", function(e) { + info("Got load on the frame: " + pending); + if (!--pending) { + resolve(); + } + }); + }); + }); + + is(Services.focus.focusedWindow, win, "Focus shouldn't have moved"); + win.close(); + } + ); +}); diff --git a/dom/base/test/file_bug1303838.html b/dom/base/test/file_bug1303838.html index 7e284e85f2cb..d11444a44937 100644 --- a/dom/base/test/file_bug1303838.html +++ b/dom/base/test/file_bug1303838.html @@ -6,20 +6,29 @@ Tests for tab switching on link clicks. Tests for tab switching on link clicks. + - Link 1
- Link 2
- Link 3
- Link 4
- Link 5
- Link 6
- Link 7
- Anchor Link 1
- Anchor Link 2
- Anchor Link 3
- Frame Link 1
- Frame Link 2
- Frame Link 3
+ Link 1, + Link 2, + Link 3, + Link 4, + Link 5, + Link 6, + Link 7, + Anchor Link 1, + Anchor Link 2, + Anchor Link 3, + Frame Link 1, + Frame Link 2, + Frame Link 3, diff --git a/dom/base/test/file_bug1700871.html b/dom/base/test/file_bug1700871.html new file mode 100644 index 000000000000..3bc1808c66fa --- /dev/null +++ b/dom/base/test/file_bug1700871.html @@ -0,0 +1,18 @@ + +Open in window. +
+ + +
+ diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp index 2deae153178d..80c0f601dd19 100644 --- a/dom/html/HTMLFormElement.cpp +++ b/dom/html/HTMLFormElement.cpp @@ -825,6 +825,7 @@ nsresult HTMLFormElement::SubmitSubmission( loadState->SetTriggeringPrincipal(NodePrincipal()); loadState->SetPrincipalToInherit(NodePrincipal()); loadState->SetCsp(GetCsp()); + loadState->SetAllowFocusMove(UserActivation::IsHandlingUserInput()); rv = nsDocShell::Cast(container)->OnLinkClickSync(this, loadState, false, NodePrincipal());