1462469 - Add a "Clear Cookies and Site Data" footer button to the identity popup

Status: VERIFIED FIXED

(Firefox :: Site Identity, enhancement, P1)

Description

[Johann Hofmann [:johannh]]

See https://mozilla.invisionapp.com/share/RSIY1B8GMC2#/screens/297843369

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1462469 - Add a "Clear Cookies and Site Data" footer button to the identity popup.

This implements a new button in the identity popup that allows users to quickly
remove cookies and site data from the sites they're visiting.

This use the SiteDataManager behind the scenes and is similar to the changes
we made for PageInfo already.

There's a major drawback to this approach in that SiteDataManager needs to
refresh its entire data set everytime we want information about a single site
or want to remove anything (it's not trivial to get rid of that limitation while
dealing with all the quirks of our storage APIs). I will work around this by
implementing a way for SiteDataManager to incrementally update itself in bug 1460768.

Review commit: https://reviewboard.mozilla.org/r/244912/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244912/

Comment 2

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aba80e756050435d75718be089b9ada8b322e03c

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8976819 [details]
Bug 1462469 - Add a "Clear Cookies and Site Data" footer button to the identity popup.

https://reviewboard.mozilla.org/r/244912/#review251828

Thanks! Tested quickly and everything seems to work fine :)

::: browser/base/content/test/siteIdentity/browser_identityPopup_clearSiteData.js:25
(Diff revision 1)
> +    SiteDataTestUtils.addToCookies(TEST_ORIGIN, "test1", "1");
> +    SiteDataTestUtils.addToCookies(TEST_ORIGIN, "test2", "2");
> +    SiteDataTestUtils.addToCookies(TEST_SUB_ORIGIN, "test1", "1");
> +  }
> +
> +  await BrowserTestUtils.withNewTab(TEST_ORIGIN, async function(browser) {

nit: any reason for not using an arrow function here?

::: browser/base/content/test/siteIdentity/browser_identityPopup_clearSiteData.js:49
(Diff revision 1)
> +    let clearButton = document.getElementById("identity-popup-clear-sitedata-button");
> +    ok(!clearFooter.hidden, "The clear data footer is not hidden.");
> +
> +    let cookiesCleared;
> +    if (testCookies) {
> +      cookiesCleared = TestUtils.topicObserved("cookie-changed", (subj, data) => data == "deleted");

Hmm, we added three cookies right? So shouldn't we wait for three cookie-changed notifications? I feel like we might get failures if the notification wasn't fired yet when we await cookiesCleared - we'd only be waiting on one cookie.

This might just me not understanding how cookie-changed works though :)

Alternatively, why not just use waitForCondition to wait until there are no more cookies, the same way we check quota storage was deleted?

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 8976819 [details]
Bug 1462469 - Add a "Clear Cookies and Site Data" footer button to the identity popup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/244912/diff/1-2/

Comment 5

[Johann Hofmann [:johannh]]

Comment on attachment 8976819 [details]
Bug 1462469 - Add a "Clear Cookies and Site Data" footer button to the identity popup.

https://reviewboard.mozilla.org/r/244912/#review251828

> nit: any reason for not using an arrow function here?

Egh, it's not required, right? :)

> Hmm, we added three cookies right? So shouldn't we wait for three cookie-changed notifications? I feel like we might get failures if the notification wasn't fired yet when we await cookiesCleared - we'd only be waiting on one cookie.
> 
> This might just me not understanding how cookie-changed works though :)
> 
> Alternatively, why not just use waitForCondition to wait until there are no more cookies, the same way we check quota storage was deleted?

Waiting for all 3 cookies now, good suggestion, thanks!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59118f646da4
Add a "Clear Cookies and Site Data" footer button to the identity popup. r=nhnt11

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/59118f646da4

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Johann, could you add a release note request? I think we want to mention that feature in the Nightly release notes at least. Thanks

https://wiki.mozilla.org/Release_Management/Release_Notes#How_to_nominate_a_bug_for_release_notes_addition.3F

Comment 9

[Johann Hofmann [:johannh]]

Release Note Request (optional, but appreciated)
[Why is this notable]: Users can now more easily than ever clear data from sites they're on
[Affects Firefox for Android]: Nope :\
[Suggested wording]: Added a "Clear Site Data and Cookies" button to the identity popup that allows you to reset local data from websites you're visiting, with only 3 clicks. This will not prevent cross-origin frames from tracking you online, so be sure to use Tracking Protection.
[Links (documentation, blog post, etc)]: None, though we might make one for the larger set of changes we're making to TP/Identity UI

Comment 10

[Pascal Chevrel:pascalc (PTO until April 26)]

Thanks, note added with this wording:

Added a ***Clear Site Data and Cookies*** button to the identity popup located next to  the address bar so as to provide a quick and easy process to delete local data for the visited website.

I am leaving the relnote-firefox? flag so as that it stays on our radar when we move nightly to beta.

Comment 11

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 62.0a1 (2018-05-17) on Windows 10, 64 Bit!
This bug's fix is verified with latest Beta!

Build ID 	20180712042337
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0

Comment 12

[Md. Majedul islam]

I have reproduced this bug with Nightly 62.0a1 (2018-05-17) on Ubuntu 18.04 LTS, 64 Bit!

This bug's fix is verified with the latest Beta!

Build ID 	20180712042337
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0

[testday-20180713]

Comment 13

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I can confirm comment 12, I verified using Fx 63.0a1(23-07-2018) and Fx 62.0b10, on Windows 10 x64, Ubuntu 14.04 LTS x64 and mac OS 10.13.5.