Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ephemeral Storage on 1p is not working correctly with LocalStorage #33077

Closed
ghost opened this issue Sep 17, 2023 · 4 comments · Fixed by brave/brave-core#20202
Closed

Ephemeral Storage on 1p is not working correctly with LocalStorage #33077

ghost opened this issue Sep 17, 2023 · 4 comments · Fixed by brave/brave-core#20202

Comments

@ghost
Copy link

ghost commented Sep 17, 2023

Description

This issue doesn't seem to affect cookies, but Ephemeral Storage has to be completely tested to see what's wrong with it, this issue seems mostly about other storages like Local and Session Storage, but I am not sure about other storages.

@goodov

To use Ephemeral Storage on 1p, sites have to be added to Sites that clear cookies when you close them, the problem is that when you add the site to it, and refresh, the site is not using the Ephemeral Storage right away for LocalStorage like it does for Cookies.
Same with switching from Clear on site close to Allow, the website should switch to Persistent Storage right away, but both scenarios are currently not working correctly, without either closing the site or restarting browser.

But for example:

  • If you add a site to clear on site close before open it, you will get the site working in the Ephemeral Storage, but switching to Persistent Storage is not going to happen, the site will keep using Ephemeral Storage, any change and everything will go to Ephemeral Storage. It will only work, if you close the site, and re-open it, which will clear the Ephemeral Storage and somehow will make it work, because again: refreshing doesn't switch storages anymore.

  • If you try to use Ephemeral Storage after visiting a site, it won't work, the browser seems to need a restart for the site to start using the Ephemeral Storage, if not, writing any Storage item will fail, like if it was set to Block, so you might see the site going back to 'defaults', but you can't make any change.

This is a video of what I mean, I am using https://mdn.github.io/dom-examples/web-storage/ to easily display and explain the issue, I know there is https://dev-pages.bravesoftware.com/storage/ephemeral-storage.html but needed something more visual to explain this issue better. When LocalStorage/SessionStorage, breaks, I could set cookies and IndexDB seemed not to throw any error when adding it through the console. But I still think Ephemeral Has to be tested and see if something else is failing besires the Local/Session Storages.

Brave-EpehemralStorage1p.mp4
This is what I do in the video, just in case is not clear what I am trying to show:

In the first 21 seconds of the video I show how Ephemeral Storage works fine, but that's because I added https://[*.]mdn.github.io:443 first before visiting it.

I can make changes and all, which are shown in Devtools, but are not written to the Persistent Storage.

At second 12 however, a tiny little visual bug appears in the View Site Information popup, it says Not allowed to save data to your device like if the site was in the Don't allow to save data which is obviously wrong, but that's just a small visual bug, not the biggest issue.

The issues start at second 21, because even if I change the site to Allow to save data and refresh, the site never switches to the Persistent storage.

In the past, it would easily switch storages only on refresh.
This is why at second 36 I had to close the website/tab, and re-open it, which would 'clear' the Ephemeral Storage, and therefore, somehow, allow the website to start using the Persistent Storage, and then it starts working as expected.

Now, the issue is at second 46, when I switch back to the Ephemeral Storage, and as you can see, the site gets reset to default red, but trying to change settings doesn't work LocalStorage fails completely and even manually adding a local storage item throws the error, like if data was in the 'down allow' and completely blocked.
So Devtools will show an error like this:

image

Closing the website won't work, so the only way to make Ephemeral Storage start working in the site is to [apparently] restart browser.

Restarting and using Ephemeral Storage, means the site will be red again and changes can be made, and that means that closing tab/site, so the Ephemeral Storage 'unloads' and opening the website again, will continue where the Persistent Storage was left.

So basically, the feature of Brave of being able to switch Storages for LocalStorage is not working correctly in Brave and sometimes completely breaks. Cookies seem to work fine, so it seems to be just a LocalStorage problem.

Switching Ephemeral Storage to Persistent Storage, is one of the best features of Brave, because you can have a basic/mini version of Firefox Containers inside Brave, where you can easily have two accounts logged and switch between them when used correctly.

I made this videos to show how the 'switching' works differently InPrivate mode, but not in Normal windows.
I used Twitch and the BTTV extension which uses LocalStorage to set settings, to easily display how only Normal Windows have the problem of not completely switching to Ephemeral Storage for the data.
Like it works with Cookies but not Local Storage items.

Private window

Brave-00000000.mp4

Normal window

Brave-00000464.mp4

Steps to Reproduce

  1. Add https://[*.]mdn.github.io:443 to Sites that clear cookies when you close them in brave://settings/cookies

  2. go to https://mdn.github.io/dom-examples/web-storage/ and change settings

  3. go back to brave://settings/cookies and move the site to Sites that can always use cookies or Allow

  4. Refresh site. it will not reset to 'default red' and will keep using Ephemeral Storage

  5. Site has to be closed and re-opened for Persistent Storage to work

  6. go back to brave://settings/cookies move site to Sites that clear cookies when you close them or Clear on site close and refresh site.

  7. it will go back to 'default red' but you can't change any setting and Devtools will display an error Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.

  8. Now Restart Browser and open website again, it will start using Ephemeral on 1p again.

Brave version (brave://version info)

1.60.28 Chromium: 117.0.5938.62 (Official Build) nightly (64-bit) Windows 11 22h2

@goodov
Copy link
Member

goodov commented Sep 19, 2023

I'm not sure if this actually worked as you described previously, because I also see issues with "localStorage" access error on older versions based on Chromium 116 and 115.

But things might've changed after #30090, it prolonged ES cleanup event to hold 30 seconds after a page close. This means a quick tab close/open did work previously, but now it doesn't work like that.

I'm going to merge a change that should switch storages on page reload properly.

@ghost
Copy link
Author

ghost commented Sep 19, 2023

You are right, it has never worked the way I described it, my mistake. I tested older versions, even went and installed 1.33.1 which is one of the first builds with it and it behaves exactly the same, I even see the same error with LocalStorage.
So I guess next time I will test the older version I can to make sure if it is Chromium or Brave, so my issue would be more accurate. Sorry about that!

mdn.github.io doesn't even work properly in Private windows, but for whatever reason Twitch works better as shown in my videos, so I guess using Private windows where it somehow worked better depending on website deceived me to never see this issue in almost 2 years!

So, it is good this was finally noticed and now it is getting fixed, so it works fine like it does with cookies.

Thanks for your work!

@ghost ghost changed the title Using Sites that clear cookies when you close them (Ephemeral Storage on 1p) is not working correctly in Chromium 117 Ephemeral Storage on 1p is not working correctly with LocalStorage Sep 19, 2023
@brave-builds brave-builds added this to the 1.60.x - Nightly milestone Sep 25, 2023
@LaurenWags
Copy link
Member

@goodov could you please add the needed labels for this one?

  • release-notes/include or release-notes/exclude
  • QA/Yes or QA/No

Also, if QA/Yes could you specify a test plan? Thanks!

@goodov
Copy link
Member

goodov commented Oct 9, 2023

@goodov could you please add the needed labels for this one?

  • release-notes/include or release-notes/exclude
  • QA/Yes or QA/No

Also, if QA/Yes could you specify a test plan? Thanks!

This is a small QoL change that's not noticeable in most scenarios. I've added a test that ensures new functionality works as expected, no other tests were broken/modified. No need to QA this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants