1886894 - Clear sessionStorage in browsingData.remove() for parity with "Clear Cookies and Site Data"

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P3)

Description

[James E. A. (any) [:codegeek98]]

Attachment: demo extension

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/115.0

Steps to reproduce:

1. Visit this test webpage https://webpages.uah.edu/~je0029/tmp/storage_example/
2. Set values in Session Storage, and in Local Storage; then read them back out
3. Manually do the "Clear cookies and Site Data" process for webpages.uah.edu https://support.mozilla.org/en-US/kb/clear-cookies-and-site-data-firefox#w_clear-cookies-for-the-current-website
4. Display the values in Session Storage, and in Local Storage
5. Again set values in Session Storage, and in Local Storage; then read them back out
6. Install the attached extension
7. Run the attached extension via its BrowserAction
8. Again display the values in Session Storage, and in Local Storage

Actual results:

Session Storage was empty in step (4), but was not empty in step (8).

Expected results:

Minimally: Session Storage should have been cleared by the extension (by including sessionStorage in browsingData.DataTypeSet)

Ideally: there should be some official API for WebExtensions to achieve parity with the "Clear cookies and Site Data" tool.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Toolkit::Data Sanitization' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[James E. A. (any) [:codegeek98]]

Re BugBot correction — I don't have any problems with the behavior of the "Clear Cookies and Site Data" function; I do have a problem with the behavior of the browsingData WebExtension API.

(This may be unrelated, but: I found that the browser itself doesn't seem to clear Session Storage when quitting with Ctrl+Shift+Q, when the browser is immediately re-launched afterwards.)

Comment 3

[Rob Wu [:robwu]]

Attachment: strxbrowser.zip

Updated test case for compatibility with Chrome.

Confirmed that the issue happens in Firefox 125.0a1, and that the issue does not in Chromium 123.0.6312.58. That is, sessionStorage is cleared when the localStorage option is set.

Comment 4

[Rob Wu [:robwu]]

The fact that the "localStorage" option clears sessionStorage is not documented in Chrome's documentation (https://developer.chrome.com/docs/extensions/reference/api/browsingData).

It is however a reasonable expectation that localStorage + sessionStorage are cleared together when the localStorage option is specified, as they are part of the same set of APIs (window.localStorage + window.sessionStorage).

The part of "Clear Cookies and Site Data" that clears sessionStorage is this one-liner:

• https://searchfox.org/mozilla-central/rev/0e9ea50a999420d93df0e4e27094952af48dd3b8/toolkit/components/cleardata/ClearDataService.sys.mjs#810-812

the implementation that removes all sessionStorage is similar, at:

• https://searchfox.org/mozilla-central/rev/0e9ea50a999420d93df0e4e27094952af48dd3b8/toolkit/components/cleardata/ClearDataService.sys.mjs#898

So to fix this bug, it would be as simple as:

• add a few lines that triggers "browser:purge-sessionStorage" to the clearLocalStorage method, at https://searchfox.org/mozilla-central/rev/0e9ea50a999420d93df0e4e27094952af48dd3b8/toolkit/components/extensions/parent/ext-browsingData.js#219,226-230
• add a unit test that confirms that sessionStorage is deleted. You can create a copy of the localStorage test and modify it to use sessionStorage: https://searchfox.org/mozilla-central/rev/0e9ea50a999420d93df0e4e27094952af48dd3b8/toolkit/components/extensions/test/mochitest/test_ext_browsingData_localStorage.html

Patches are welcome; to get started, see https://wiki.mozilla.org/WebExtensions/Contribution_Onramp

Comment 5

[James E. A. (any) [:codegeek98]]

Looking at the source code, I'm having trouble tracing that.

It looks like the call flow is as follows:

1. The "Clear cookies and site data..." click is handled by gIdentityHandler.clearSiteData(event)

2. That handler ultimately calls SiteDataManager.remove() https://hg.mozilla.org/mozilla-central/file/3c5105fce0360bb6708a9ac3f09d06527b316119/browser/base/content/browser-siteIdentity.js#l416

3. That function ultimately attempts to clear Cookies, Cache, DOMStorage, Storage Persistence grants, Storage Access grants, Media Extension data, and assorted built-in privacy add-on states https://hg.mozilla.org/mozilla-central/file/3c5105fce0360bb6708a9ac3f09d06527b316119/browser/modules/SiteDataManager.sys.mjs#l524

   • The cookie cleaner seems to only clear cookies https://hg.mozilla.org/mozilla-central/file/3c5105fce0360bb6708a9ac3f09d06527b316119/toolkit/components/cleardata/ClearDataService.sys.mjs#l183

   • The CLEAR_DOM_STORAGES and CLEAR_ALL_CACHES flags seem to be silently dropped, as far as I can tell 🤨 (I assume I'm misreading this?) https://hg.mozilla.org/mozilla-central/file/3c5105fce0360bb6708a9ac3f09d06527b316119/toolkit/components/cleardata/ClearDataService.sys.mjs#l2060

I don't see how any method of QuotaCleaner would get called based on a "Clear cookies and site data..." action. What am I missing?

Comment 6

[James E. A. (any) [:codegeek98]]

As an aside: I'm less looking to achieve parity with Chrome as I am hoping to build an extension that "Clears cookies and site data" for only the default Tab Container on browser shutdown or other user request. I certainly don't have any preference over whether localStorage and sessionStorage are lumped together in the same flag like Chrome, or managed as separate flags. (I don't have any Chrome-based browser to hand anyway, so I didn't realize they already provided extensions the ability to remove sessionStorage.)

Setting aside Chrome, then, does browsingData.remove() have any other known deficiencies relative to SiteDataManager.remove()?

Comment 7

[Jim Gong]

Hello there! I was able to implement the needed functionality, but when I tried to add a test (copy test_ext_browsingData_localStorage.html test and change localStorage inside contentScript to seesionStorage), the test would fail. Anytime checkLocalStorageSet event is sent, sessionStorage.getItem wasn't able to pick up the set value. Was wondering if this has to do with some sort of racing problem, as sessionStorage is not globally available.
Would appreciate any directions on how to fix the test. Thank you!

Comment 8

[Jim Gong]

Attachment: Bug 1886894 - removeLocalStorage now removes both localStorage and sessionStorage, a unit test is added. r?robwu!

Comment 9

[Rob Wu [:robwu]]

I see that you've unblocked yourself before I had a chance to respond here; I'll respond in the patch after doing a code review.

Comment 10

[Jim Gong]

Attachment: Bug 1886894 - Skip originScope check when the site name is empty. r?robwu!

Comment 11

[Jim Gong]

Attachment: Bug 1886894 - add extension:purge-sessionStorage event to do exact domain match when domain is provided. r?robwu!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a315c687a539
https://hg.mozilla.org/integration/autoland/rev/31a780fc1272
removeLocalStorage now removes both localStorage and sessionStorage, a unit test is added. r=robwu
https://github.com/mozilla-firefox/firefox/commit/642862cc0053
https://hg.mozilla.org/integration/autoland/rev/518885b3270a
Add clearAllForPattern attribute to nsIClearBySiteEntry Interface to clear sessionStorage accross all origins with a given pattern. r=robwu,dom-storage-reviewers,asuth
https://github.com/mozilla-firefox/firefox/commit/32759256a6cf
https://hg.mozilla.org/integration/autoland/rev/07de3a0de652
add DomainMatchingMode enum so browser:purge-sessionStorage does exact domain match when domain is provided. r=robwu,dom-storage-reviewers,edenchuang

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9ee356d4f4b0
https://hg.mozilla.org/integration/autoland/rev/8b7372980447
Revert "Bug 1886894 - add DomainMatchingMode enum so browser:purge-sessionStorage does exact domain match when domain is provided. r=robwu,dom-storage-reviewers,edenchuang" for causing bc failure on /browser_sessionStorage.js

Comment 14

[Narcis Beleuzu [:NarcisB]]

Backed out for causing bc failure on /browser_sessionStorage.js

Backout link: https://hg-edge.mozilla.org/integration/autoland/rev/8b7372980447a1b850b51d585d01dfd731b5cc5c
Log link: https://treeherder.mozilla.org/logviewer?job_id=530364328&repo=autoland&task=IwCf1sIwQbiWWZm5fIx-dQ.0&lineNumber=38979

Comment 15

[Jim Gong]

(In reply to Narcis Beleuzu [:NarcisB] from comment #14)

Backed out for causing bc failure on /browser_sessionStorage.js

Backout link: https://hg-edge.mozilla.org/integration/autoland/rev/8b7372980447a1b850b51d585d01dfd731b5cc5c
Log link: https://treeherder.mozilla.org/logviewer?job_id=530364328&repo=autoland&task=IwCf1sIwQbiWWZm5fIx-dQ.0&lineNumber=38979

Thank you for the information!
Looks like the test is directly interacting with deleteBySite at https://searchfox.org/firefox-main/rev/da7a647e48e4c1ff687cd1d17fa89ce78ddc14fa/toolkit/components/cleardata/ClearDataService.sys.mjs#902

The newly updated behaviour of browser:purge-sessionStorage with site passed in this patch would do an exact site name check instead of clearing every sub-domain. So the two failed tests are expected behaviour. I was thinking if it is okay for me to update the tests with comments.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1e66975721ed
https://hg.mozilla.org/integration/autoland/rev/5aa4c6c9660c
removeLocalStorage now removes both localStorage and sessionStorage, a unit test is added. r=robwu
https://github.com/mozilla-firefox/firefox/commit/c3f7c6c25567
https://hg.mozilla.org/integration/autoland/rev/bd3fe9cfeb05
Skip originScope check when the site name is empty. r=robwu,dom-storage-reviewers,asuth
https://github.com/mozilla-firefox/firefox/commit/3428dbfba2c8
https://hg.mozilla.org/integration/autoland/rev/2364607169e7
add extension:purge-sessionStorage event to do exact domain match when domain is provided. r=robwu,dom-storage-reviewers,edenchuang

Comment 17

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5aa4c6c9660c
https://hg.mozilla.org/mozilla-central/rev/bd3fe9cfeb05
https://hg.mozilla.org/mozilla-central/rev/2364607169e7

Comment 18

[Rob Wu [:robwu]]

dev-doc-needed: browsingData.removeLocalStorage (and the remove method with the localStorage option) now also clears sessionStorage, not just localStorage. The documentation refers to "localStorage" (sometimes "local storage"), these should also mention sessionStorage.

We should also drop the note about cookieStoreId support being Nightly-only, as it has been supported for years at this point: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browsingData/RemovalOptions#cookiestoreid

Comment 19

[rbloor]

Documentation updates available in:

• (content) Bug 1886894 Clear sessionStorage in browsingData remove options #41609
• (BCD) Bug 1886894 Clear sessionStorage in browsingData remove options #28261