Malte Kraus Reporter
	Description • 9 years ago

Clicking a link with target=_blank from a popup window in private browsing mode opens a new tab in a window that is not in private browsing mode.

Steps to reproduce:
- open Firefox
- open a new private window
- open the attached html file in the private window
- click the link "click 1", a popup opens (still marked private browsing)
- in the popup, click the link "click 2"
- notice that the link opened in the non-private window. (If there are no non-private windows, clicking the link does nothing at all.)


This does not happen when e10s is disabled.

	Alice0775 White
	Comment 1 • 9 years ago

I can also reproduce on Windows7.

	Alice0775 White
	Comment 2 • 9 years ago

Pushlog;
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=68b90a5407b2&tochange=8798cd000e6b

Suspect: Bug 567058

	Alice0775 White
	Comment 3 • 9 years ago

Via local build,
Last Good: 613d80a116b4
First Bad: 8798cd000e6b
 
Regressed by: 8798cd000e6b	Bill McCloskey — Bug 567058 - Stop using intr messages for window.open (r=bent)

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 4 • 9 years ago

Hey Blake - can I take this off your hands?

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 5 • 9 years ago

Assuming yes - ping me if you'd like it back!

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 6 • 9 years ago

Bug 1202634 - Make sure TabParent LoadContext for pop-up shares private browsing state of opener. r?billm

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 7 • 9 years ago

Comment on attachment 8662560 [details]
MozReview Request: Bug 1202634 - Make sure TabParent LoadContext for pop-ups shares private browsing state of opener. r=billm

So the problem here appears to be that the chrome flags on the TabParent aren't getting the private browsing / remote flags set properly on them.

I think I'm partially responsible for this due to my patch in bug 1114299, which makes it so that the content process cannot send up remoteness / private browsing flags. I had added that rule because it looked like the parent was "doing the right thing", and setting the private browsing state on the newly opened windows properly. What I didn't realize is that TabParent's get their own LoadContext's, and store the chrome flags that were used to open the new TabParent, and those chrome flags need to have the private browsing flag set if content from a private browsing window requests a new window be opened.

I... I think this is the right move. But I'm not 100% certain. Window opening is kinda hard, it seems. :/

billm, do you have any feedback on this?

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 8 • 9 years ago

Comment on attachment 8662560 [details]
MozReview Request: Bug 1202634 - Make sure TabParent LoadContext for pop-ups shares private browsing state of opener. r=billm

Bug 1202634 - Make sure TabParent LoadContext for pop-up shares private browsing state of opener. r?billm

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 9 • 9 years ago

Bug 1202634 - Regression test. r?jdm

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 10 • 9 years ago

Comment on attachment 8662560 [details]
MozReview Request: Bug 1202634 - Make sure TabParent LoadContext for pop-ups shares private browsing state of opener. r=billm

The regression test is probably landable, but I'm still not sure about this bit.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 11 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=52f48fec7fd2

	Josh Matthews [:jdm]
	Comment 12 • 9 years ago

Comment on attachment 8663766 [details]
MozReview Request: Bug 1202634 - Regression test. r=jdm

https://reviewboard.mozilla.org/r/19853/#review18073

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 13 • 9 years ago

Comment on attachment 8662560 [details]
MozReview Request: Bug 1202634 - Make sure TabParent LoadContext for pop-ups shares private browsing state of opener. r=billm

https://reviewboard.mozilla.org/r/19639/#review18245

::: dom/ipc/nsIContentParent.cpp:134
(Diff revision 2)
> +  uint32_t chromeFlagsCopy = aChromeFlags;

Just call this chromeFlags.

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 14 • 9 years ago

Comment on attachment 8662560 [details]
MozReview Request: Bug 1202634 - Make sure TabParent LoadContext for pop-ups shares private browsing state of opener. r=billm

Bug 1202634 - Make sure TabParent LoadContext for pop-ups shares private browsing state of opener. r=billm

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 15 • 9 years ago

Comment on attachment 8663766 [details]
MozReview Request: Bug 1202634 - Regression test. r=jdm

Bug 1202634 - Regression test. r=jdm

	Pulsebot
	Comment 16 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f9433f93a56
https://hg.mozilla.org/integration/mozilla-inbound/rev/71e494cbdd1d

	Carsten Book [:Tomcat]
	Comment 17 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/6f9433f93a56
https://hg.mozilla.org/mozilla-central/rev/71e494cbdd1d

	Mohammad Maruf Islam
	Comment 18 • 9 years ago

I have reproduced this bug with Firefox Nightly 43.0a1 (Build ID: 20150908030203) on 
windows 8.1 64-bit with the instructions from comment 0 .

Verified as fixed with Firefox Nightly 44.0a1 (Build ID: 20151023030245

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0

	Mike Conley (:mconley) (:⚙️) Assignee
	Comment 19 • 8 years ago
bugnotes

http://people.mozilla.org/~mconley2/bugnotes/bug-1202634.html