Bug 1801761 - Generate focus actionId in SetFocusInner rather than relying on the caller to provide it r=emilio,hsivonen

This fixes an issue where Element::Focus triggered another focus
event and caused the initial focus event failed to be executed.

Differential Revision: https://phabricator.services.mozilla.com/D163156
This commit is contained in:
Sean Feng 2023-02-15 20:08:37 +00:00
parent 8e0155cf74
commit 40ee61069d
2 changed files with 41 additions and 35 deletions

View File

@ -380,8 +380,7 @@ nsFocusManager::GetActiveBrowsingContext(BrowsingContext** aBrowsingContext) {
void nsFocusManager::FocusWindow(nsPIDOMWindowOuter* aWindow,
CallerType aCallerType) {
if (RefPtr<nsFocusManager> fm = sInstance) {
fm->SetFocusedWindowWithCallerType(aWindow, aCallerType,
sInstance->GenerateFocusActionId());
fm->SetFocusedWindowWithCallerType(aWindow, aCallerType);
}
}
@ -402,19 +401,19 @@ nsFocusManager::GetFocusedContentBrowsingContext(
}
nsresult nsFocusManager::SetFocusedWindowWithCallerType(
mozIDOMWindowProxy* aWindowToFocus, CallerType aCallerType,
uint64_t aActionId) {
LOGFOCUS(("<<SetFocusedWindow begin actionid: %" PRIu64 ">>", aActionId));
mozIDOMWindowProxy* aWindowToFocus, CallerType aCallerType) {
LOGFOCUS(("<<SetFocusedWindow begin>>"));
nsCOMPtr<nsPIDOMWindowOuter> windowToFocus =
nsPIDOMWindowOuter::From(aWindowToFocus);
NS_ENSURE_TRUE(windowToFocus, NS_ERROR_FAILURE);
nsCOMPtr<Element> frameElement = windowToFocus->GetFrameElementInternal();
Maybe<uint64_t> actionIdFromSetFocusInner;
if (frameElement) {
// pass false for aFocusChanged so that the caret does not get updated
// and scrolling does not occur.
SetFocusInner(frameElement, 0, false, true, aActionId);
actionIdFromSetFocusInner = SetFocusInner(frameElement, 0, false, true);
} else {
// this is a top-level window. If the window has a child frame focused,
// clear the focus. Otherwise, focus should already be in this frame, or
@ -428,19 +427,21 @@ nsresult nsFocusManager::SetFocusedWindowWithCallerType(
}
nsCOMPtr<nsPIDOMWindowOuter> rootWindow = windowToFocus->GetPrivateRoot();
const uint64_t actionId = actionIdFromSetFocusInner.isSome()
? actionIdFromSetFocusInner.value()
: sInstance->GenerateFocusActionId();
if (rootWindow) {
RaiseWindow(rootWindow, aCallerType, aActionId);
RaiseWindow(rootWindow, aCallerType, actionId);
}
LOGFOCUS(("<<SetFocusedWindow end actionid: %" PRIu64 ">>", aActionId));
LOGFOCUS(("<<SetFocusedWindow end actionid: %" PRIu64 ">>", actionId));
return NS_OK;
}
NS_IMETHODIMP nsFocusManager::SetFocusedWindow(
mozIDOMWindowProxy* aWindowToFocus) {
return SetFocusedWindowWithCallerType(aWindowToFocus, CallerType::System,
GenerateFocusActionId());
return SetFocusedWindowWithCallerType(aWindowToFocus, CallerType::System);
}
NS_IMETHODIMP
@ -470,7 +471,7 @@ nsFocusManager::SetFocus(Element* aElement, uint32_t aFlags) {
NS_ENSURE_ARG(aElement);
SetFocusInner(aElement, aFlags, true, true, GenerateFocusActionId());
SetFocusInner(aElement, aFlags, true, true);
LOGFOCUS(("<<SetFocus end>>"));
@ -543,7 +544,7 @@ nsFocusManager::MoveFocus(mozIDOMWindowProxy* aWindow, Element* aStartElement,
// would be a problem because the caret would move to the beginning of the
// focused link making it impossible to navigate the caret over a link.
SetFocusInner(MOZ_KnownLive(newFocus->AsElement()), aFlags,
aType != MOVEFOCUS_CARET, true, GenerateFocusActionId());
aType != MOVEFOCUS_CARET, true);
*aElement = do_AddRef(newFocus->AsElement()).take();
} else if (aType == MOVEFOCUS_ROOT || aType == MOVEFOCUS_CARET) {
// no content was found, so clear the focus for these two types.
@ -1472,14 +1473,15 @@ static bool IsEmeddededInNoautofocusPopup(BrowsingContext& aBc) {
.GetXULBoolAttr(nsGkAtoms::noautofocus);
}
void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
bool aFocusChanged, bool aAdjustWidget,
uint64_t aActionId) {
Maybe<uint64_t> nsFocusManager::SetFocusInner(Element* aNewContent,
int32_t aFlags,
bool aFocusChanged,
bool aAdjustWidget) {
// if the element is not focusable, just return and leave the focus as is
RefPtr<Element> elementToFocus =
FlushAndCheckIfFocusable(aNewContent, aFlags);
if (!elementToFocus) {
return;
return Nothing();
}
const RefPtr<BrowsingContext> focusedBrowsingContext =
@ -1515,7 +1517,7 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
// focused rather than the frame it is in.
if (!newWindow || (newBrowsingContext == GetFocusedBrowsingContext() &&
elementToFocus == mFocusedElement)) {
return;
return Nothing();
}
MOZ_ASSERT(newBrowsingContext);
@ -1532,7 +1534,7 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
BrowsingContext* walk = focusedBrowsingContext;
while (walk) {
if (walk == bc) {
return;
return Nothing();
}
walk = walk->GetParent();
}
@ -1549,13 +1551,13 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
bool inUnload;
docShell->GetIsInUnload(&inUnload);
if (inUnload) {
return;
return Nothing();
}
bool beingDestroyed;
docShell->IsBeingDestroyed(&beingDestroyed);
if (beingDestroyed) {
return;
return Nothing();
}
BrowsingContext* bc = docShell->GetBrowsingContext();
@ -1570,7 +1572,7 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
do {
bc = bc->GetParent();
if (bc && bc->IsDiscarded()) {
return;
return Nothing();
}
} while (bc && !bc->IsInProcess());
if (bc) {
@ -1616,12 +1618,12 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
}
if (!focusedPrincipal || !newPrincipal) {
return;
return Nothing();
}
if (!focusedPrincipal->Subsumes(newPrincipal)) {
NS_WARNING("Not allowed to focus the new window!");
return;
return Nothing();
}
}
@ -1728,11 +1730,12 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
LOGFOCUS((" Flags: %x Current Window: %p New Window: %p Current Element: %p",
aFlags, mFocusedWindow.get(), newWindow.get(),
mFocusedElement.get()));
const uint64_t actionId = GenerateFocusActionId();
LOGFOCUS(
(" In Active Window: %d Moves to different BrowsingContext: %d "
"SendFocus: %d actionid: %" PRIu64,
isElementInActiveWindow, focusMovesToDifferentBC, sendFocusEvent,
aActionId));
actionId));
if (sendFocusEvent) {
Maybe<BlurredElementInfo> blurredInfo;
@ -1785,19 +1788,19 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
? focusedBrowsingContext.get()
: nullptr),
commonAncestor, focusMovesToDifferentBC, aAdjustWidget,
remainActive, aActionId, elementToFocus)) {
return;
remainActive, actionId, elementToFocus)) {
return Some(actionId);
}
}
Focus(newWindow, elementToFocus, aFlags, focusMovesToDifferentBC,
aFocusChanged, false, aAdjustWidget, aActionId, blurredInfo);
aFocusChanged, false, aAdjustWidget, actionId, blurredInfo);
} else {
// otherwise, for inactive windows and when the caller cannot steal the
// focus, update the node in the window, and raise the window if desired.
if (allowFrameSwitch) {
AdjustWindowFocus(newBrowsingContext, true, IsWindowVisible(newWindow),
aActionId);
actionId);
}
// set the focus node and method as needed
@ -1829,7 +1832,7 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
RaiseWindow(outerWindow,
aFlags & FLAG_NONSYSTEMCALLER ? CallerType::NonSystem
: CallerType::System,
aActionId);
actionId);
} else {
mozilla::dom::ContentChild* contentChild =
mozilla::dom::ContentChild::GetSingleton();
@ -1838,11 +1841,12 @@ void nsFocusManager::SetFocusInner(Element* aNewContent, int32_t aFlags,
aFlags & FLAG_NONSYSTEMCALLER
? CallerType::NonSystem
: CallerType::System,
aActionId);
actionId);
}
}
}
}
return Some(actionId);
}
static already_AddRefed<BrowsingContext> GetParentIgnoreChromeBoundary(

View File

@ -214,8 +214,7 @@ class nsFocusManager final : public nsIFocusManager,
* Setter for focusedWindow with CallerType
*/
MOZ_CAN_RUN_SCRIPT nsresult SetFocusedWindowWithCallerType(
mozIDOMWindowProxy* aWindowToFocus, mozilla::dom::CallerType aCallerType,
uint64_t aActionId);
mozIDOMWindowProxy* aWindowToFocus, mozilla::dom::CallerType aCallerType);
/**
* Given an element, which must be the focused element, activate the remote
@ -314,10 +313,13 @@ class nsFocusManager final : public nsIFocusManager,
*
* All actual focus changes must use this method to do so. (as opposed
* to those that update the focus in an inactive window for instance).
*
* Returns Nothing() if we end up not trying to focus the element,
* otherwise returns the generated action id.
*/
MOZ_CAN_RUN_SCRIPT void SetFocusInner(mozilla::dom::Element* aNewContent,
int32_t aFlags, bool aFocusChanged,
bool aAdjustWidget, uint64_t aActionId);
MOZ_CAN_RUN_SCRIPT Maybe<uint64_t> SetFocusInner(
mozilla::dom::Element* aNewContent, int32_t aFlags, bool aFocusChanged,
bool aAdjustWidget);
/**
* Returns true if aPossibleAncestor is the same as aWindow or an