Electrolysis: Make content.opener work

RESOLVED FIXED in Firefox 41

Status

()

Firefox
Tabbed Browser
P3
normal
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: billm, Assigned: mconley)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 41
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm7+, firefox41 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments, 1 obsolete attachment)

The browser code frequently checks if content.opener is non-null. We should be able to forward this information from the content process.
This seems to magically work due to CPOWs - ie, the scratchpad in e10s builds tells me content.opener is "[object XrayWrapper [object Window]]" and I can fetch properties on it (eg, content.opener.location.href is correct, etc).

Is there something more subtle I'm missing?
That works, but is of course not really nice, because we have to create all these wrappers just for such a simple check.
Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".
tracking-e10s: --- → +
Blocks: 997462

Updated

4 years ago
Blocks: 691608
Assignee: nobody → wmccloskey
Priority: -- → P3
No longer blocks: 997462
Whiteboard: [e10s-m3]
Moving to M3 milestone. These bugs affect e10s dogfooding but Brad and Gavin did not think they were M2 blockers.
tracking-e10s: + → m3+
Moving this up to m5+. This doesn't really have much user-visible effect at all.
tracking-e10s: m3+ → m5+
m5+
Whiteboard: [e10s-m3]
Assignee: wmccloskey → mconley
Status: NEW → ASSIGNED
I took this, but having read it, I don't think it qualifies as an M5. Re-nomming.
Status: ASSIGNED → NEW
tracking-e10s: m5+ → ?

Updated

3 years ago
tracking-e10s: ? → m7+
Created attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r a8b9112c5d9954ceebbece2a28dbaba403f89c16 https://reviewboard-hg.mozilla.org/gecko/
Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r a8b9112c5d9954ceebbece2a28dbaba403f89c16 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8603393 - Flags: review?(felipc)
https://reviewboard.mozilla.org/r/8419/#review7109

When a tab is navigated to a different page, the content.opener information will change, right? So the cached value must be updated. Here the initial value for a tab will remain throughout its existence.
When I test single-process Firefox, tabs with openers (like one opened from target="_blank") seem to keep that opener for the lifetime of the outer browser window... do you see that too?

I was trying to emulate that behaviour. I've never even read the spec behind opener - not sure what the "right" thing to do here is.
Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r d7d1c65a573aef2f27a5d67c627c0e958b77938f https://reviewboard-hg.mozilla.org/gecko/
Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

https://reviewboard.mozilla.org/r/8417/#review7203

::: browser/base/content/browser.js:4174
(Diff revisions 1 - 2)
> +        Pocket.onLocationChange(browser, aLocationURI);

is this part of the patch?
Attachment #8603393 - Flags: review?(felipc)
https://reviewboard.mozilla.org/r/8417/#review7261

> is this part of the patch?

Nope, this Review Board not understanding that this patch series was rebased. Sorry about that. :)
Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

https://reviewboard.mozilla.org/r/8417/#review7263

Ship It!
Attachment #8603393 - Flags: review+
Ok, I've gone through my notes on how we do window opening and tab opening, and I think I know how we can pull this off more safely. New patch coming up.
Attachment #8603393 - Flags: review+ → review?(felipc)
Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8723 - Bug 863515 - Expose hasContentOpener on nsITabParent. r=?
/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r 73cd053d96985c5cb1667a8821a419f3a6641600 https://reviewboard-hg.mozilla.org/gecko/
Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8723 - Bug 863515 - Expose hasContentOpener on nsITabParent. r=?
/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=?
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=?

Pull down these commits:

hg pull -r 73cd053d96985c5cb1667a8821a419f3a6641600 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8603393 - Flags: review?(bugs)
Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley

/r/8723 - Bug 863515 - Expose hasContentOpener on nsITabParent. r=?
/r/8419 - Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=felipe.
/r/8421 - Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=felipe.

Pull down these commits:

hg pull -r e53fb43afdd10ca6d8dff970abdaa82cb0848ca7 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8603393 - Flags: review?(felipc)

Updated

3 years ago
Attachment #8603393 - Flags: review?(bugs) → review+
update iid of nsITabParent
https://hg.mozilla.org/mozilla-central/rev/fcee8fe97ec8
https://hg.mozilla.org/mozilla-central/rev/fa82c160f259
https://hg.mozilla.org/mozilla-central/rev/b8965a7ca29d
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment on attachment 8603393 [details]
MozReview Request: bz://863515/mconley
Attachment #8603393 - Attachment is obsolete: true
Attachment #8618011 - Flags: review+
Attachment #8618012 - Flags: review+
Attachment #8618013 - Flags: review+
Created attachment 8618011 [details]
MozReview Request: Bug 863515 - Expose hasContentOpener on nsITabParent. r=?
Created attachment 8618012 [details]
MozReview Request: Bug 863515 - Add property to browser bindings to indicate whether content caused it to open. r=felipe.
Created attachment 8618013 [details]
MozReview Request: Bug 863515 - Switch usages of content.opener to gBrowser.selectedBrowser.hasContentOpener. r=felipe.
You need to log in before you can comment on or make changes to this bug.