Closed Bug 1073826 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 35
Tracking Status
firefox35 --- verified
fennec 35+ ---

People

(Reporter: c.ascheberg, Assigned: billm)

References

()

Details

(Keywords: regression, reproducible)

Attachments

(1 file)

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
tracking-fennec: --- → ?
Flags: in-testsuite?
Keywords: reproducible
I'll grab a window this weekend
QA Contact: aaron.train
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)
Blocks: 1069059
Flags: needinfo?(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.
Flags: needinfo?(margaret.leibovic) → needinfo?(wmccloskey)
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.
Likely the same issue, content from a private tab is opened in the background into a new regular tab, but thanks for reporting.
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.
Attached patch fix-androidSplinter Review
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.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8497079 - Flags: review?(margaret.leibovic)
Flags: needinfo?(wmccloskey)
Comment on attachment 8497079 [details] [diff] [review]
fix-android

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

Thanks for the fix.
Attachment #8497079 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/3e5a4c8504bc
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
tracking-fennec: ? → 35+
Verified as fixed in build 35.0a1 (2014-10-03);
Device: Nexus 7 (Android 4.4.4).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.