Closed Bug 1031273 Opened 6 years ago Closed 5 years ago

"Clear Private Data" does not clear "Recently closed tabs" from Recent Tabs

Categories

(Firefox for Android :: General, defect)

33 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 34
Tracking Status
firefox33 --- verified
firefox34 --- verified
fennec 33+ ---

People

(Reporter: TeoVermesan, Assigned: liuche)

References

Details

(Keywords: reproducible)

Attachments

(1 file, 2 obsolete files)

Device: Samsung Galaxy Nexus
OS: Android 4.2.2
Build: Firefox for Android 33.0a1 (2014-06-26)

Steps to reproduce:
1. Open a few tabs and close them (the tabs now appear in the "Recently closed tabs" list from Recent Tabs
2. Go to Settings -> Privacy -> Clear Private Data (with all options checked)
3. Go to Recent Tab panel

Expected results:
- The list should be empty

Actual results:
- The tabs closed at step 1 are still displayed in the "Recently closed tabs" list
I was thinking of filing this the other day, but I think this is intentional (maybe? maybe not?). The tabs are cleared regardless on next browser startup if you opt to not restore them.
We probably just need to update the sanitize/session store code to handle this case. Or to notify the recent tabs panel properly when the closed tabs list is cleared. Without looking at the code, I suspect this is caused by a missing notification.
Blocks: 1004850
tracking-fennec: --- → ?
Assignee: nobody → liuche
tracking-fennec: ? → 33+
This is probably caused by the "mClosedTabs.length > 0" condition in RecentTabsPanel; see https://bugzilla.mozilla.org/show_bug.cgi?id=1035439#c11. If so, this should be fixed by bug 1035439.
I can still reproduce this on Nightly 07/16. The list is only sanitized after a clear if I close the active about:home tab and visit it again in a new one.
Status: NEW → ASSIGNED
Keywords: reproducible
This doesn't handle clearing Recent Tabs if you click the "Clear History" button from the History panel - it's not in the scope of this bug, but it seems like we'd like to do that too.
Attachment #8480961 - Flags: review?(margaret.leibovic)
Comment on attachment 8480961 [details] [diff] [review]
Patch: Add handling for Sanitize:ClearHistory

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

::: mobile/android/base/home/RecentTabsPanel.java
@@ +247,5 @@
> +                    }
> +                }
> +            });
> +        } else if ("Sanitize:ClearHistory".equals(event)) {
> +            getLoaderManager().restartLoader(LOADER_ID_RECENT_TABS, null, mCursorLoaderCallbacks);

Does this properly update recently closed tabs as well as tabs from last time? You may need to clear out mClosedTabs as well. Although actually, shouldn't SessionStore.js sending us an updated set of tabs here with a "ClosedTabs:Data" message? Did you look into what messages we're getting from SessionStore.js? Looking into this right now, I think I found the bug.

I think the real fix is to just make sure we call _sendClosedTabsToJava in here (surrounded by a |if (this._notifyClosedTabs)| check):
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/SessionStore.js#109
Attached patch Patch: Clear recent tabs (obsolete) — Splinter Review
Good call! This does clear recent tabs as well as tabs from last time (which is what I was checking).
Attachment #8480961 - Attachment is obsolete: true
Attachment #8480961 - Flags: review?(margaret.leibovic)
Attachment #8480981 - Flags: review?(margaret.leibovic)
Comment on attachment 8480981 [details] [diff] [review]
Patch: Clear recent tabs

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

r+ with comment addressed. Thanks!

::: mobile/android/components/SessionStore.js
@@ +116,5 @@
>            this.saveState();
>          }
>  
>          Services.obs.notifyObservers(null, "sessionstore-state-purge-complete", "");
> +        this._sendClosedTabsToJava(Services.wm.getMostRecentWindow("navigator:browser"));

You should wrap this in an |if (this._notifyClosedTabs)| check.

I think this should also fix the clear history button case. But if it doesn't we can file a follow-up for that.
Attachment #8480981 - Flags: review?(margaret.leibovic) → review+
Derp, I just forgot...

https://hg.mozilla.org/integration/fx-team/rev/ef4e47eaadd9
Target Milestone: --- → Firefox 34
https://hg.mozilla.org/mozilla-central/rev/ef4e47eaadd9
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Carrying over r+ (for aurora patch).

Approval Request Comment
[Feature/regressing bug #]: Recent Tabs home panel
[User impact if declined]: Users won't see the correct state of Recent Tabs after clearing private data until opening a new instance of the home panel
[Describe test coverage new/current, TBPL]: local testing
[Risks and why]: very low, adding a callback call that was omitted in the original patch
[String/UUID change made/needed]: none
Attachment #8480981 - Attachment is obsolete: true
Attachment #8481409 - Flags: review+
Attachment #8481409 - Flags: approval-mozilla-aurora?
Attachment #8481409 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
The list is empty after clearing private data, so verified as fixed on:
Builds: Firefox for Android 34.0a1 (2014-08-31) and Firefox for Android 33.0a2 (2014-08-31)
Device: Alcatel One Touch (Android 4.1.2)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.