1554065 - nsDocShell::DoFindItemWithName seems to pass the wrong requestor to the TabGroup

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Boris Zbarsky [:bzbarsky]]

The whole point of that aRequestor argument to TabGroup::FindItemWithName is to indicate where we've already searched for named windows. But right now, if we start a search in some docshell and call nsDocShell::DoFindItemWithName, we will pass its aRequestor to the tabgroup. That doesn't seem right to me; seems like we should be passing this instead. As things stand, we end up calling back into that toplevel docshell from the tabgroup and searching everything under there again; we only avoid infinite-looping because for that call aSkipTabGroup is set to true.

It looks to me like this bug got introduced in bug 1337537 (and in particular in https://hg.mozilla.org/mozilla-central/rev/3092fa33385e44cef90c76ba00c46eb9439797f2).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1554065. Fix the requestor nsDocShell::DoFindItemWithName passes to TabGroup::FindItemWithName. r=nika

The requestor should be ourselves: the toplevel docshell that the tabgroup does
not need to look in when doing the search.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a0bbb60153c
Fix the requestor nsDocShell::DoFindItemWithName passes to TabGroup::FindItemWithName.  r=nika

Comment 3

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/1a0bbb60153c

Comment 4

[Daniel Bodea [:danibodea]]

Hi. Considering that the regressor was pushed in firefox54, can we set firefox68 and firefox67 as affected? Thanks.

Comment 5

[Boris Zbarsky [:bzbarsky]]

It really doesn't matter too much. Apart from being slightly inefficient, this problem is not observable, so there's no plan to backport the fix.