Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] Clearing private tabs from notification closes the app #9100

Closed
sv-ohorvath opened this issue Mar 11, 2020 · 7 comments
Closed

[Bug] Clearing private tabs from notification closes the app #9100

sv-ohorvath opened this issue Mar 11, 2020 · 7 comments
Assignees
@BranescuMihai
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified eng:ready Ready for engineering Feature:PrivateBrowsing needs:triage Issue needs triage S1 Blocks development/testing, may impact more than 25% of users, causes data loss, potential chemspill
Projects
Sawyer's Features Hershey's 🍫

Comments

@sv-ohorvath
Copy link
Contributor

sv-ohorvath commented Mar 11, 2020 •
edited by data-sync-user
Loading

Steps to reproduce

  1. Open a tab in private browsing.
  2. Tap home to minimize the app
  3. Tap the private tabs notification from the system notification tray.

Expected behavior

Private tabs are closed.

Actual behavior

App gets removed from recent apps.

Device information

  • Android device:
  • Fenix version: Nightly 3/11

┆Issue is synchronized with this Jira Task
@sv-ohorvath sv-ohorvath added 🐞 bug Crashes, Something isn't working, .. Feature:PrivateBrowsing S1 Blocks development/testing, may impact more than 25% of users, causes data loss, potential chemspill labels Mar 11, 2020
@sv-ohorvath sv-ohorvath mentioned this issue Mar 11, 2020
Closed
@github-actions github-actions bot added the needs:triage Issue needs triage label Mar 11, 2020
FoxDie12x pushed a commit to FoxDie12x/fenix that referenced this issue Mar 18, 2020
Closes mozilla-mobile#9100 (bug): 'clearing private tabs from notific…
e2e5fd7 
…ation closes the app'
@FoxDie12x
Copy link

I would like to take a go on this

@FoxDie12x FoxDie12x mentioned this issue Mar 19, 2020
Closed
4 tasks
@BranescuMihai
Copy link
Contributor

The expected behaviour is that the private tabs are closed AND we are redirected to PB home screen right?

This bug is highly connected with #8590 in the sense that there's one fix for both.

There are two approaches that I can think of for the PB notification, one simpler than the other, and the difference is this:
When the app is in background and you click the notification, should the PB home pop up in foreground(easy) or not(still doable, but more effort)? I assume it's the second option.

@BranescuMihai BranescuMihai added the needs:UX-feedback Needs UX Feedback label Mar 20, 2020
@brampitoyo
Copy link

@BranescuMihai the expected behaviour is as follows.

There are two situations when users can tap on the “Close private tabs” notification.

  1. If notification is tapped while Firefox is in background
    • All private tabs are closed
    • Firefox is not removed from the Recent Apps list
    • The notification shade disappears, but Firefox is still minimised
    • When reopened, Firefox should show normal Homescreen (not PB Homescreen)
  2. If notification is tapped while Firefox is in foreground
    • All private tabs are closed
    • Firefox is not removed from the Recent Apps list
    • The notification shade disappears, and Firefox shows up again
    • Firefox should show normal Homescreen (not PB Homescreen)

The only difference between the two scenarios is: if Firefox is in background, we keep it there – don’t reopen it. If Firefox is in foreground, we will show it again.

Either way, the page that should be shown is not the PB Homescreen, but the normal Homescreen

@brampitoyo brampitoyo self-assigned this Apr 1, 2020
FoxDie12x pushed a commit to FoxDie12x/fenix that referenced this issue Apr 2, 2020
For mozilla-mobile#9100 (and probably mozilla-mobile#8590): fixes 'cl…
d66f50e 
…earing private tabs from notification closes the app'
FoxDie12x pushed a commit to FoxDie12x/fenix that referenced this issue Apr 2, 2020
For mozilla-mobile#9100 (and likely mozilla-mobile#8590): fixes 'clea…
f690d02 
…ring private tabs from notification closes the app' (+ cleanup)
@liuche
Copy link
Contributor

liuche commented Apr 10, 2020

I realized that Bram is not available for Easter, so for now, I think since we have a patch, we can fix this particular bug according to what Bram suggested, and then later we can revisit whether #8590 is still important.

I'll flag Bram for the case that we're not addressing here is the one where a user is using Private Mode from the Private Browsing shortcut, which we made as a "Focus replacement", in #8590.

BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue Apr 15, 2020
@BranescuMihai
For mozilla-mobile#9100 - Fixed private browsing button
e63c708 
When you clicked it, it would send your task to recent apps, removed the intent extra that was triggering this
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue Apr 15, 2020
@BranescuMihai
Closes mozilla-mobile#9100 (bug): 'clearing private tabs from notific…
9d6920f 
…ation closes the app'
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue Apr 15, 2020
@BranescuMihai
For mozilla-mobile#9100 (and probably mozilla-mobile#8590): fixes 'cl…
7c2548d 
…earing private tabs from notification closes the app'
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue Apr 15, 2020
@BranescuMihai
For mozilla-mobile#9100 (and likely mozilla-mobile#8590): fixes 'clea…
e698bb7 
…ring private tabs from notification closes the app' (+ cleanup)
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue Apr 15, 2020
@BranescuMihai
For mozilla-mobile#9100 - Fixed private browsing button
fa3e2da 
When you clicked it, it would send your task to recent apps, removed the intent extra that was triggering this
@ekager ekager mentioned this issue Apr 17, 2020
Closed
@sblatz sblatz added this to Private Mode in Sawyer's Features May 13, 2020
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue May 20, 2020
@BranescuMihai
Closes mozilla-mobile#9100 (bug): 'clearing private tabs from notific…
583b968 
…ation closes the app'
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue May 20, 2020
@BranescuMihai
For mozilla-mobile#9100 (and probably mozilla-mobile#8590): fixes 'cl…
b3a8e2b 
…earing private tabs from notification closes the app'
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue May 20, 2020
@BranescuMihai
For mozilla-mobile#9100 (and likely mozilla-mobile#8590): fixes 'clea…
6925171 
…ring private tabs from notification closes the app' (+ cleanup)
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 20, 2020
@BranescuMihai
For mozilla-mobile#9100 - Fixed private browsing button
5ac3ce4 
When you clicked it, it would send your task to recent apps, removed the intent extra that was triggering this
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue May 20, 2020
@BranescuMihai
For mozilla-mobile#9100 - private notification redirection
51c1bcd 
We redirect to private mode if the entry point is one of the private shortcuts.
We redirect to normal mode if the entry point is any other
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue May 22, 2020
@BranescuMihai
Closes mozilla-mobile#9100 (bug): 'clearing private tabs from notific…
469ba17 
…ation closes the app'
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue May 22, 2020
@BranescuMihai
For mozilla-mobile#9100 (and probably mozilla-mobile#8590): fixes 'cl…
97f3fe2 
…earing private tabs from notification closes the app'
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue May 22, 2020
@BranescuMihai
For mozilla-mobile#9100 (and likely mozilla-mobile#8590): fixes 'clea…
bef2fb3 
…ring private tabs from notification closes the app' (+ cleanup)
@BranescuMihai BranescuMihai self-assigned this Jun 11, 2020
@BranescuMihai BranescuMihai moved this from In Progress to In Dev Review in Hershey's 🍫 Jun 11, 2020
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue Jun 24, 2020
@BranescuMihai
Closes mozilla-mobile#9100 (bug): 'clearing private tabs from notific…
116a207 
…ation closes the app'
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue Jun 24, 2020
@BranescuMihai
For mozilla-mobile#9100 (and probably mozilla-mobile#8590): fixes 'cl…
438be93 
…earing private tabs from notification closes the app'
BranescuMihai pushed a commit to BranescuMihai/fenix that referenced this issue Jun 24, 2020
@BranescuMihai
For mozilla-mobile#9100 (and likely mozilla-mobile#8590): fixes 'clea…
497a5de 
…ring private tabs from notification closes the app' (+ cleanup)
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue Jun 24, 2020
@BranescuMihai
For mozilla-mobile#9100 - Fixed private browsing button
9e11e57 
When you clicked it, it would send your task to recent apps, removed the intent extra that was triggering this
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue Jun 24, 2020
@BranescuMihai
For mozilla-mobile#9100 - private notification redirection
1d5b086 
We redirect to private mode if the entry point is one of the private shortcuts.
We redirect to normal mode if the entry point is any other
ekager pushed a commit to BranescuMihai/fenix that referenced this issue Jun 25, 2020
@ekager
Closes mozilla-mobile#9100 (bug): 'clearing private tabs from notific…
2496df3 
…ation closes the app'
ekager pushed a commit to BranescuMihai/fenix that referenced this issue Jun 25, 2020
@ekager
For mozilla-mobile#9100 (and probably mozilla-mobile#8590): fixes 'cl…
68f3681 
…earing private tabs from notification closes the app'
ekager pushed a commit to BranescuMihai/fenix that referenced this issue Jun 25, 2020
@ekager
For mozilla-mobile#9100 (and likely mozilla-mobile#8590): fixes 'clea…
e77a5ff 
…ring private tabs from notification closes the app' (+ cleanup)
ekager pushed a commit to BranescuMihai/fenix that referenced this issue Jun 25, 2020
@BranescuMihai @ekager
For mozilla-mobile#9100 - Fixed private browsing button
815c9e6 
When you clicked it, it would send your task to recent apps, removed the intent extra that was triggering this
ekager pushed a commit to BranescuMihai/fenix that referenced this issue Jun 25, 2020
@BranescuMihai @ekager
For mozilla-mobile#9100 - private notification redirection
de59be8 
We redirect to private mode if the entry point is one of the private shortcuts.
We redirect to normal mode if the entry point is any other
ekager pushed a commit that referenced this issue Jun 25, 2020
@BranescuMihai
For #9100 - Private browsing notification fixes
49b617c 
Co-authored-by: Seef <Saif Dara>
@ekager ekager added the eng:qa:needed QA Needed label Jun 25, 2020
@AndiAJ
Copy link
Collaborator

AndiAJ commented Jun 25, 2020

Hi, verified as fixed on the latest Nightly Build 200625 from 6/25 using the following devices:
• Google Pixel 3a (Android 10)
• Huawei Mate 20 Lite (Android 9)
• OnePlus A3 (Android 6.0.1)

► Video
20200625-094917

@AndiAJ AndiAJ closed this as completed Jun 25, 2020
Sawyer's Features automation moved this from Private Mode to Done Jun 25, 2020
Hershey's 🍫 automation moved this from In Dev Review to QA Review Jun 25, 2020
@AndiAJ AndiAJ added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Jun 25, 2020
@mcarare mcarare moved this from QA Review to Done in Hershey's 🍫 Jun 25, 2020
csadilek added a commit to csadilek/fenix that referenced this issue Jun 25, 2020
@csadilek
For mozilla-mobile#9100: Follow-up to fix memory leak in Notification…
ef6d736 
…SessionObserver

The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
csadilek added a commit to csadilek/fenix that referenced this issue Jun 25, 2020
@csadilek
For mozilla-mobile#9100: Follow-up to fix memory leak in Notification…
ae388fc 
…SessionObserver

The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
csadilek added a commit to csadilek/fenix that referenced this issue Jun 25, 2020
@csadilek
For mozilla-mobile#9100: Follow-up to fix memory leak in Notification…
fceb051 
…SessionObserver

The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
csadilek added a commit to csadilek/fenix that referenced this issue Jun 25, 2020
@csadilek
For mozilla-mobile#9100: Follow-up to fix memory leak in Notification…
26411a0 
…SessionObserver

The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
csadilek added a commit to csadilek/fenix that referenced this issue Jun 25, 2020
@csadilek
For mozilla-mobile#9100: Follow-up to fix memory leak in Notification…
df21a7f 
…SessionObserver

The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
csadilek added a commit to csadilek/fenix that referenced this issue Jun 25, 2020
@csadilek
For mozilla-mobile#9100: Follow-up to fix memory leak in Notification…
cf1d3c5 
…SessionObserver

The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
csadilek added a commit to csadilek/fenix that referenced this issue Jun 25, 2020
@csadilek
For mozilla-mobile#9100: Follow-up to fix memory leak in Notification…
e4ec999 
…SessionObserver

The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
csadilek added a commit that referenced this issue Jun 25, 2020
@csadilek
For #9100: Follow-up to fix memory leak in NotificationSessionObserver
6444040 
The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
@liuche liuche mentioned this issue Jun 27, 2020
Closed
12 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@BranescuMihai BranescuMihai

Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified eng:ready Ready for engineering Feature:PrivateBrowsing needs:triage Issue needs triage S1 Blocks development/testing, may impact more than 25% of users, causes data loss, potential chemspill
Projects
No open projects
Hershey's 🍫
  
Done
Sawyer's Features
  
Done
Milestone
No milestone
Development

No branches or pull requests
8 participants
@brampitoyo @liuche @FoxDie12x @BranescuMihai @sv-ohorvath @AmyYLee @ekager @AndiAJ