Electrolysis: link targeting across content/chrome

RESOLVED FIXED

Status

()

Core
Document Navigation
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Benjamin Smedberg, Assigned: bz)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

9 years ago
Link targeting needs to be handled correctly across content/chrome: new windows may need to be redirected into tabs by the chrome process; named targets may need to be resolved to existing windows by the chrome process (and that navigation may cause the window to be rendered by a different content process).
OK, so general thoughts:

1)  If link targeting happens via window.open, the resulting window is returned
    to the caller so must be in the same process, no?
2)  If link targeting happens via target="" then the resulting window gets its
    .opener set to the caller; again must be in the same process.

So it seems to me that we can guarantee that link/window.open targeting need only examine windows in the same process.  Agreed?  If not, then what are we doing about .opener and the return value of window.open?
(Reporter)

Comment 2

9 years ago
Yes, in general. In fact for the Fennec case we only need to have one content process overall. What does need to work is that new windows need to go through the parent process in such a way that Fennec can open new tabs, and Firefox can open popup windows/tabs for them. Allowing nsIBrowserDOMWindow to work, I think?

From the chromium docs: "There is one exception: Chromium allows pages to fork a new rendering process via JavaScript, in a way that is compatible with the links that appear in Gmail messages. A page can open a new tab to about:blank, set the new tab's window.opener  variable to null, and then redirect the new tab to a cross-site URL. In that case, a renderer-initiated navigation will cause Chromium to switch renderer processes in the new tab. Thus, opening a link from Gmail (or pages with similar logic) will not adversely affect the Gmail process."
Created attachment 416233 [details] [diff] [review]
Add a method to fennec's nsIBrowserDOMWindow implementation to add a tab and return its browser
Created attachment 416234 [details] [diff] [review]
Hack fennec's tab-opening code to not throw
Created attachment 416236 [details] [diff] [review]
Implement nsIInterfaceRequestor on TabChild, so the nsDocShellTreeOwner can get things off it
Attachment #416236 - Flags: review?(benjamin)
Created attachment 416237 [details] [diff] [review]
Add a new API to nsIBrowserDOMWindow to like openURI but returning a frame loader owner

I'm not totally convinced of this API.  In particular, my use case doesn't need aURI or aOwner.  It does need the other two, sort of; what I'm passing now isn't quite right.
Attachment #416237 - Flags: review?(benjamin)
Created attachment 416238 [details] [diff] [review]
Make it work

I can't visually inspect it, since fennec's barfing on tab switches, for example, but I checked in a debugger that ProvideWindow returns a window succesfully.
Attachment #416238 - Flags: review?(benjamin)
(Reporter)

Updated

9 years ago
Attachment #416236 - Flags: review?(benjamin) → review+
(Reporter)

Updated

9 years ago
Attachment #416237 - Flags: review?(benjamin) → review+
(Reporter)

Updated

9 years ago
Attachment #416238 - Flags: review?(benjamin) → review+
(Reporter)

Comment 8

9 years ago
I have no good opinions about the XXX comments in these patches, by the way: I suspect that we should make a more invasive change to nsIBrowserDOMWindow so that there aren't two APIs, so make sure followup bugs are filed if necessary.
Filed bug 537428 on sorting out the right nsIBrowserDOMWindow API, as well as some icky details regarding modal content windows and the like.

Filed bug 537429 on the TabChild GetInterface question.

Pushed:
http://hg.mozilla.org/mozilla-central/rev/5d75d1ef28c1
http://hg.mozilla.org/mozilla-central/rev/15b58ed34cb0
http://hg.mozilla.org/mozilla-central/rev/1e25778852f2
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.