1554070 - Named targeting can target bfcached subframes

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

Consider this testcase, involving two same-origin files test1.html and test2.html:

test1.html:

    <iframe name="foo"></iframe>
    <a href="test2.html">Click me</a>

test2.html:

    <a target="foo" href="https://example.org">Click me</a>

And perform the following steps:

1. Load test1.html.
2. Click the link (to test2.html).
3. Click the link on test2.html.

EXPECTED RESULTS: New window (or tab) opens and loads example.org

ACTUAL RESULTS: Nothing happens.

What ends up happening is that when we unload test1.html it goes into bfcache. Its subframe docshells get removed from the docshell tree and stashed in the history entry.

Now when we try to target the link, we land in TabGroup::FindItemWithName. This walks the list of all the windows in the tabgroup, which includes the subframe window! That window has no parent, and its docshell has no parent (since it's all unhooked from the docshell tree), so we treat it as a root window and end up calling FindItemWithName on that docshell, which finds a match (the docshell itself). We then try to do the load in there instead of treading this as an unknown target.

I checked, and this has been broken for a while. Definitely years. Possibly all the way back to 1303196. I can't run nightlies from that long ago, apparently. :(

Anyway, in terms of fixes... We could test docshell->IsFrame() on the docshell we're about to try searching. If that returns true (which in this case it does), then this is a docshell that really should have a parent but currently doesn't because of bfcache or teardown or something and we should not search it.

We could also test outerWindow->IsFrozen() and not search if that comes back true, right? I guess it depends on what happens if we start freezing background tabs...

Thoughts on what the right test here is?

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Hmm - good point. I think the best answer here is probably to check docshell->IsFrame() rather than checking if the docshell has no parent.

Alternatively, we can also check if the BrowsingContext has no parent - the BrowsingContext's parent edge isn't cleared when in the bfcache, unlike the docshell's.

Comment 2

[u608768]

Attachment: Bug 1554070 - Ensure that the selected window has a targetable BrowsingContext, r=nika

When searching for a target by name, don't select a window if its associated
BrowsingContext is closed, discarded, or cached.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07572fb43cf7
Ensure that the selected window has a targetable BrowsingContext, r=nika

Comment 4

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/07572fb43cf7