1073826 - clicking a link that opens a in new tab while in private browsing opens a regular tab

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Christian Ascheberg]

STR:
- go to private browsing mode
- click a link that automatically opens in a new tab

Result:
- clicked link loads in a regular tab

Expected:
- link should be opened in private browsing mode

Comment 1

[Aaron Train [:aaronmt]]

I'll grab a window this weekend

Comment 2

[Aaron Train [:aaronmt]]

Last good revision: 1e2993c99323 (2014-09-24)
First bad revision: 1735ff2bb23e (2014-09-25)

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1e2993c99323&tochange=1735ff2bb23e

My money is on this refactoring 

324798b60ba3	Bill McCloskey — Bug 1069059 - [e10s] Convert isWindowPrivate calls to isBrowserPrivate/isContentWindowPrivate as appropriate (r=mconley,margaret)

Comment 3

[:Margaret Leibovic]

It looks like this line is the one that would be at fault:
https://hg.mozilla.org/mozilla-central/rev/324798b60ba3#l8.51

Bill, should this have stayed isWindowPrivate? I'm not sure of the implementation differences of those methods.

Comment 4

[Christian Ascheberg]

Selecting a text on a website and then clicking search regressed in the same way. I did not file a separate bug (yet) because I do not know if it is the very same bug.

Comment 5

[Aaron Train [:aaronmt]]

Likely the same issue, content from a private tab is opened in the background into a new regular tab, but thanks for reporting.

Comment 6

[:Margaret Leibovic]

Actually, I was wrong, this line is the culprit:
https://hg.mozilla.org/mozilla-central/rev/324798b60ba3#l8.70

Reverting this fixed the issue for me locally, so it appears this is an issue with isBrowserPrivate.

Looking at the implementation of isBrowserPrivate, I think it actually doesn't do the right thing for Fennec, since all of Fennec's <browser> elements are children of the same <deck> in a single XUL window (as opposed to desktop, which has private windows).

So I think we should revert these isBrowserPrivate changes in Fennec, or come up with an alternate helper method we can use.

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: fix-android

I actually like the idea of having isBrowserPrivate, since it fits the Fennec usage pretty well. Rather than revert the previous patch, it seems better to restrict its usage to e10s. That's what this patch does.

Using gMultiProcessBrowser in toolkit/ it a little hacky, but I think it's nicer than adding a Fennec-specific check.

Thanks for the help Margaret.

Comment 8

[:Margaret Leibovic]

Comment on attachment 8497079 [details] [diff] [review]
fix-android

Review of attachment 8497079 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the fix.

Comment 9

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e5a4c8504bc

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3e5a4c8504bc

Comment 11

[Flaviu Cos]

Verified as fixed in build 35.0a1 (2014-10-03);
Device: Nexus 7 (Android 4.4.4).