From 0fd119dae058d89769ffa1d0ee75fe31679bcd90 Mon Sep 17 00:00:00 2001 From: Edgar Chen Date: Mon, 21 Dec 2020 21:35:49 +0000 Subject: [PATCH] Bug 582459 - Reset the focused element when a frame loses document focus; r=mikedeboer,NeilDeakin,hsivonen,jaws Resetting focus would also clear selection on editable element, so get current selected text before moving focus to findbar to make prefill-with-selection work if the content is loaded in chrome process. Differential Revision: https://phabricator.services.mozilla.com/D89557 --- .../chrome/test_confirm_delete_dialog.html | 6 +-- dom/base/nsFocusManager.cpp | 39 +++++++++------ dom/events/test/mochitest.ini | 1 - dom/events/test/test_bug450876.html | 2 +- dom/tests/mochitest/chrome/window_focus.xhtml | 6 +-- layout/reftests/selection/reftest.list | 2 +- ...iveelement-after-focusing-out-iframes.html | 50 +++++++++++++++++++ ...-out-different-site-iframes-outer.sub.html | 34 +++++++++++++ ...ment-after-focusing-out-iframes-inner.html | 38 ++++++++++++++ ...-focusing-out-same-site-iframes-outer.html | 34 +++++++++++++ .../focus/focus-management/focus-events.html | 1 + toolkit/content/widgets/findbar.js | 3 +- toolkit/modules/Finder.jsm | 2 +- 13 files changed, 192 insertions(+), 26 deletions(-) create mode 100644 testing/web-platform/tests/focus/iframe-activeelement-after-focusing-out-iframes.html create mode 100644 testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-different-site-iframes-outer.sub.html create mode 100644 testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-iframes-inner.html create mode 100644 testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-same-site-iframes-outer.html diff --git a/browser/components/aboutlogins/tests/chrome/test_confirm_delete_dialog.html b/browser/components/aboutlogins/tests/chrome/test_confirm_delete_dialog.html index d9d64c2ce7c9..0339bd109f9e 100644 --- a/browser/components/aboutlogins/tests/chrome/test_confirm_delete_dialog.html +++ b/browser/components/aboutlogins/tests/chrome/test_confirm_delete_dialog.html @@ -109,15 +109,15 @@ add_task(async function test_dialog_focus_trap() { ok(displayElChildSpan.tabIndex === -1, "The tabIndex value for elements with a hardcoded tabIndex attribute should be reset to '-1'.") ok(displayElChildSpan.dataset.oldTabIndex === "0", "Existing tabIndex values should be stored in `dataset.oldTabIndex`.") - const isActiveElemDialogOrHTML = (elemTagName) => { - return (["HTML", "CONFIRMATION-DIALOG"].includes(elemTagName)); + const isActiveElemDialogOrHTMLorBODY = (elemTagName) => { + return (["HTML", "BODY", "CONFIRMATION-DIALOG"].includes(elemTagName)); } let iterator = 0; while(iterator < 20) { sendKey("TAB"); isnot(document.activeElement.id, "display-child", "The display-child element should not gain focus when the dialog is showing"); - is(isActiveElemDialogOrHTML(document.activeElement.tagName), true, "The confirmation-dialog should always have focus when the dialog is showing"); + ok(isActiveElemDialogOrHTMLorBODY(document.activeElement.tagName), "The confirmation-dialog should always have focus when the dialog is showing"); iterator++; } }); diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp index 08c35f59d0e9..e39cef1aab98 100644 --- a/dom/base/nsFocusManager.cpp +++ b/dom/base/nsFocusManager.cpp @@ -1626,13 +1626,6 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags, } // return if blurring fails or the focus changes during the blur if (focusedBrowsingContext) { - // if the focus is being moved to another element in the same document, - // or to a descendant, pass the existing window to Blur so that the - // current node in the existing window is cleared. If moving to a - // window elsewhere, we want to maintain the current node in the - // window but still blur it. - bool currentIsSameOrAncestor = - IsSameOrAncestor(focusedBrowsingContext, newWindow); // find the common ancestor of the currently focused window and the new // window. The ancestor will need to have its currently focused node // cleared once the document has been blurred. Otherwise, we'll be in a @@ -1650,11 +1643,28 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags, commonAncestor = GetCommonAncestor(newWindow, focusedBrowsingContext); } - if (!Blur( - currentIsSameOrAncestor ? focusedBrowsingContext.get() : nullptr, - commonAncestor ? commonAncestor.get() : nullptr, - focusMovesToDifferentBC, aAdjustWidget, aActionId, - elementToFocus)) { + bool needToClearFocusedElement = false; + if (focusedBrowsingContext->IsChrome()) { + // Always reset focused element if focus is currently in chrome window. + needToClearFocusedElement = true; + } else { + // Only reset focused element if focus moves within the same top-level + // content window. + if (focusedBrowsingContext->Top() == browsingContextToFocus->Top()) { + // XXX for the case that we try to focus an + // already-focused-remote-frame, we would still send blur and focus + // IPC to it, but they will not generate blur or focus event, we don't + // want to reset activeElement on the remote frame. + needToClearFocusedElement = (focusMovesToDifferentBC || + focusedBrowsingContext->IsInProcess()); + } + } + + if (!Blur(needToClearFocusedElement ? focusedBrowsingContext.get() + : nullptr, + commonAncestor ? commonAncestor.get() : nullptr, + focusMovesToDifferentBC, aAdjustWidget, aActionId, + elementToFocus)) { return; } } @@ -3545,11 +3555,12 @@ nsresult nsFocusManager::DetermineElementToMoveFocus( docShell->TabToTreeOwner(forward, forDocumentNavigation, &tookFocus); // If the tree owner took the focus, blur the current element. if (tookFocus) { - nsCOMPtr window = docShell->GetWindow(); - if (window->GetFocusedElement() == mFocusedElement) { + if (GetFocusedBrowsingContext() && + GetFocusedBrowsingContext()->IsInProcess()) { Blur(GetFocusedBrowsingContext(), nullptr, true, true, GenerateFocusActionId()); } else { + nsCOMPtr window = docShell->GetWindow(); window->SetFocusedElement(nullptr); } return NS_OK; diff --git a/dom/events/test/mochitest.ini b/dom/events/test/mochitest.ini index 0361effefa4c..670055c6b749 100644 --- a/dom/events/test/mochitest.ini +++ b/dom/events/test/mochitest.ini @@ -59,7 +59,6 @@ skip-if = verify [test_bug448602.html] [test_bug450876.html] skip-if = verify -fail-if = (xorigin && fission) [test_bug456273.html] [test_bug457672.html] skip-if = toolkit == 'android' #CRASH_DUMP, RANDOM diff --git a/dom/events/test/test_bug450876.html b/dom/events/test/test_bug450876.html index c90e5fc411e4..1f522a9e792d 100644 --- a/dom/events/test/test_bug450876.html +++ b/dom/events/test/test_bug450876.html @@ -31,7 +31,7 @@ function doTest() { is(document.activeElement, document.getElementById('a'), "link should have focus"); is(document.hasFocus(), true, "document should be focused"); synthesizeKey("KEY_Tab"); - is(document.activeElement, document.getElementById('a'), "body element should be focused"); + is(document.activeElement, document.body, "body element should be focused"); is(document.hasFocus(), false, "document should not be focused"); SimpleTest.finish(); diff --git a/dom/tests/mochitest/chrome/window_focus.xhtml b/dom/tests/mochitest/chrome/window_focus.xhtml index aa536dea0b92..7bf7b5acd529 100644 --- a/dom/tests/mochitest/chrome/window_focus.xhtml +++ b/dom/tests/mochitest/chrome/window_focus.xhtml @@ -1448,8 +1448,7 @@ function doFrameSwitchingTests() [framed.contentDocument, "blur", null, null, window, null], [framed.contentWindow, "blur", null, null, window, null], [frameb.contentDocument, "focus", null, frameb.contentWindow, window, frameb], - [frameb.contentWindow, "focus", null, frameb.contentWindow, window, frameb], - [inputb, "focus", inputb, frameb.contentWindow, window, frameb]]; + [frameb.contentWindow, "focus", null, frameb.contentWindow, window, frameb]]; fm.focusedWindow = frameb.contentWindow; ok(gEventMatched && gExpectedEvents.length == 0, "frame switch using focusedWindow"); @@ -1461,8 +1460,7 @@ function doFrameSwitchingTests() // focus a sibling frame by setting focusedWindow when no element is focused in that frame gEventMatched = true; - gExpectedEvents = [[inputb, "blur", null, frameb.contentWindow, window, frameb], - [frameb.contentDocument, "blur", null, null, window, null], + gExpectedEvents = [[frameb.contentDocument, "blur", null, null, window, null], [frameb.contentWindow, "blur", null, null, window, null], [framec.contentDocument, "focus", null, framec.contentWindow, window, framea], [framec.contentWindow, "focus", null, framec.contentWindow, window, framea]]; diff --git a/layout/reftests/selection/reftest.list b/layout/reftests/selection/reftest.list index c397a867f42a..a3038f7da065 100644 --- a/layout/reftests/selection/reftest.list +++ b/layout/reftests/selection/reftest.list @@ -53,5 +53,5 @@ needs-focus == semitransparent-decoration-line.html semitransparent-decoration-l fuzzy-if(OSX,0-1,0-6) fuzzy-if(Android,0-188,0-39) needs-focus == writing-mode.html writing-mode-ref.html needs-focus == 1478604.html 1478604-ref.html -needs-focus != disabled-1.html disabled-1-notref.html +needs-focus == disabled-1.html disabled-1-notref.html needs-focus != disabled-2.html disabled-2-notref.html diff --git a/testing/web-platform/tests/focus/iframe-activeelement-after-focusing-out-iframes.html b/testing/web-platform/tests/focus/iframe-activeelement-after-focusing-out-iframes.html new file mode 100644 index 000000000000..832b2675e254 --- /dev/null +++ b/testing/web-platform/tests/focus/iframe-activeelement-after-focusing-out-iframes.html @@ -0,0 +1,50 @@ + + +iframe activeElement after focusing out iframe + + + diff --git a/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-different-site-iframes-outer.sub.html b/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-different-site-iframes-outer.sub.html new file mode 100644 index 000000000000..3fcebfc8ba4c --- /dev/null +++ b/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-different-site-iframes-outer.sub.html @@ -0,0 +1,34 @@ + + +iframe active element after focusing out different site iframes outer +
+ + diff --git a/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-iframes-inner.html b/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-iframes-inner.html new file mode 100644 index 000000000000..a94af6b9fbae --- /dev/null +++ b/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-iframes-inner.html @@ -0,0 +1,38 @@ + + + + + iframe active element inner document + + +

Inner


+ + + + diff --git a/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-same-site-iframes-outer.html b/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-same-site-iframes-outer.html new file mode 100644 index 000000000000..98399deb7b61 --- /dev/null +++ b/testing/web-platform/tests/focus/support/iframe-activeelement-after-focusing-out-same-site-iframes-outer.html @@ -0,0 +1,34 @@ + + +iframe active element after focusing out same site iframes outer +
+ + diff --git a/testing/web-platform/tests/html/interaction/focus/focus-management/focus-events.html b/testing/web-platform/tests/html/interaction/focus/focus-management/focus-events.html index d63362aaa182..5f8ca20a13d8 100644 --- a/testing/web-platform/tests/html/interaction/focus/focus-management/focus-events.html +++ b/testing/web-platform/tests/html/interaction/focus/focus-management/focus-events.html @@ -25,6 +25,7 @@ assert_true(e.isTrusted, "blur event is trusted"); assert_false(e.bubbles, "blur event doesn't bubble"); assert_false(e.cancelable, "blur event is not cancelable"); + assert_equals(document.activeElement, document.body); }); i1.focus(); diff --git a/toolkit/content/widgets/findbar.js b/toolkit/content/widgets/findbar.js index 8fd764a2f5b1..a1bd3fd58b22 100644 --- a/toolkit/content/widgets/findbar.js +++ b/toolkit/content/widgets/findbar.js @@ -1147,6 +1147,8 @@ } if (this.prefillWithSelection && userWantsPrefill) { + this.browser.finder.getInitialSelection(); + // NB: We have to focus this._findField here so tests that send // key events can open and close the find bar synchronously. this._findField.focus(); @@ -1157,7 +1159,6 @@ // jumbled up queries. this._findField.select(); - this.browser.finder.getInitialSelection(); return startFindPromise; } diff --git a/toolkit/modules/Finder.jsm b/toolkit/modules/Finder.jsm index 4e1c599bb7ad..bf544fe09246 100644 --- a/toolkit/modules/Finder.jsm +++ b/toolkit/modules/Finder.jsm @@ -374,8 +374,8 @@ Finder.prototype = { }, getInitialSelection() { + let initialSelection = this.getActiveSelectionText().selectedText; this._getWindow().setTimeout(() => { - let initialSelection = this.getActiveSelectionText().selectedText; for (let l of this._listeners) { try { l.onCurrentSelection(initialSelection, true);