Closed
Bug 516747
Opened 15 years ago
Closed 15 years ago
Electrolysis: link targeting across content/chrome
Categories
(Core :: DOM: Navigation, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: benjamin, Assigned: bzbarsky)
References
Details
Attachments
(5 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•15 years ago
|
||
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•15 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."
Assignee | ||
Comment 3•15 years ago
|
||
Assignee | ||
Comment 4•15 years ago
|
||
Assignee | ||
Comment 5•15 years ago
|
||
Attachment #416236 -
Flags: review?(benjamin)
Assignee | ||
Comment 6•15 years ago
|
||
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)
Assignee | ||
Comment 7•15 years ago
|
||
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•15 years ago
|
Attachment #416236 -
Flags: review?(benjamin) → review+
Reporter | ||
Updated•15 years ago
|
Attachment #416237 -
Flags: review?(benjamin) → review+
Reporter | ||
Updated•15 years ago
|
Attachment #416238 -
Flags: review?(benjamin) → review+
Reporter | ||
Comment 8•15 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.
Assignee | ||
Comment 9•15 years ago
|
||
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
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•