Closed Bug 1339066 Opened 3 years ago Closed 3 years ago

"Open in Private Tab" option seems to display the website in a new tab during normal session

Categories

(Firefox for Android :: Awesomescreen, defect, P1)

All
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 54
Tracking Status
fennec 53+ ---
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- verified
firefox54 --- verified

People

(Reporter: carmenf, Assigned: twointofive)

References

Details

(Keywords: regression)

Attachments

(2 files)

Environment: 
Devices:  
- Huawei MediaPad M2 (Android 5.1.1);
- Lenovo Yoga Tablet 2 (Android 4.4.2);
- Nexus 9 (Android 7.1.1)
Build: latest Nightly 54.0a1 (2017-02-13);

Steps to reproduce:
1. Open Firefox;
2. Go to about:home page;
3. Long tap on one of the websites displayed to trigger the context menu;
4. Tap on "Open in Private Tab" option.

Expected result:
Snackbar is displayed stating the following: "New private tab opened|SWITCH"
The website is loaded in a new private tab.

Actual result:
The website seems to be loaded in a new tab while browser is on normal session, but the tab counter number remains the same (is not increasing). By tapping the tab, the user is redirected to the website in private session.

Notes:
The website is not displayed in the History list.
Please see the Screencast attached.
Since the first affected version is 53, I wonder whether the event dispatcher changes (bug 1329268 et al) are to blame again?
tracking-fennec: --- → ?
Tested with Huawei MediaPad M2 (Android 5.1.1)

Regression Window performed:
Last known good build: 2016-12-19
First known bad build: 2016-12-20

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=863c2b61bd27bb6099104933134d3be7c052551a&tochange=567894f026558e6dada617a3998f29aed06ac7d8
Bug 1317446 seems to be the most likely candidate in that range.
Blocks: 1317446
Flags: needinfo?(twointofive)
Hardware: ARM → All
Yup: we used to update the entire tab strip list of tabs each time a new tab was added; when I switched to just updating the adapter with only the new tab I didn't bring over the privacy check that the mass update used to do.
Assignee: nobody → twointofive
Flags: needinfo?(twointofive)
The pre-RV version of addTab, for reference: https://hg.mozilla.org/mozilla-central/file/916b9e55af42/mobile/android/base/java/org/mozilla/gecko/tabs/TabStripView.java#l292

I'll plan on uplifting this one to aurora.
I wound up adding 250ms sleep to the test to give time for the UI to update in the case where we do fail, in order to prevent false positives - the issue here is that the UI shouldn't change at all when we're non-buggy (except for a snackbar which is displayed at the same time the new tab gets created in js), so there's nothing in the UI to wait on in the working case (we were already waiting on Content:DOMTitleChanged, but that doesn't necessarily imply UI is updated).  With the buggy code the test was failing for me even without the sleep, but... Am I being too paranoid here?
tracking-fennec: ? → 53+
Priority: -- → P1
Comment on attachment 8838982 [details]
Bug 1339066 - Don't add a private tab opened while viewing the normal-mode tab strip.

https://reviewboard.mozilla.org/r/113734/#review116082
Attachment #8838982 - Flags: review?(s.kaspari) → review+
Pushed by twointofive@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/212bad17569e
Don't add a private tab opened while viewing the normal-mode tab strip. r=sebastian
https://hg.mozilla.org/mozilla-central/rev/212bad17569e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Please request Aurora approval on this when you get a chance.
Flags: needinfo?(twointofive)
Comment on attachment 8838982 [details]
Bug 1339066 - Don't add a private tab opened while viewing the normal-mode tab strip.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1317446
[User impact if declined]: On tablets, if a user is in normal mode (as opposed to private mode) and opens a link in a new private tab, the private tab will be added to the normal mode tab strip.
[Is this code covered by automated tests?]: No.  (The patch includes a tablet-only test, but we don't run automated tests on tablets.)
[Has the fix been verified in Nightly?]: I verified the fix in Nightly.
[Needs manual test from QE? If yes, steps to reproduce]: Steps to reproduce are in comment 0 - tablet only.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: The fix is to check the privacy state of a tab before adding it to the tab strip, which is what used to happen before the regression.
[String changes made/needed]: None.
Attachment #8838982 - Flags: approval-mozilla-aurora?
Flags: needinfo?(twointofive)
Comment on attachment 8838982 [details]
Bug 1339066 - Don't add a private tab opened while viewing the normal-mode tab strip.

Fix for private browsing regression on android, let's take it on aurora.
Attachment #8838982 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Tested on latest Nightly build 54.0a1 (2017-02-24) and latest Aurora 53.0a2 (2017-02-24) using following devices:
- Asus ZenPad 8.0 (Android 6.0.1);
- Nexus 9 (Android 7.1.1).

Snackbar is displayed stating the following: "New private tab opened|SWITCH"
The website is loaded in a new private tab, without being displayed in the normal session.

I'm marking this bug as Verified for Firefox 54 and 53.
You need to log in before you can comment on or make changes to this bug.