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
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: --- → ?
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)
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.
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)
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+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Verified as fixed in build 35.0a1 (2014-10-03); Device: Nexus 7 (Android 4.4.4).
You need to log in before you can comment on or make changes to this bug.