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

Don't throw an exception when accessing local,sessionStorage in 3p frame #9758

Closed
pes10k opened this issue May 12, 2020 · 6 comments · Fixed by brave/brave-core#5533
Closed
Assignees
Labels

Comments

@pes10k
Copy link
Contributor

pes10k commented May 12, 2020

Currently, Chromium throws an exception when JS just touches window.{local,session}Storage, which prevents sites from doing typical feature detection style checks. We should just act as if these values aren't present (e.g. window.localStorage === undefined|null) so that defensive / well programmed sites still work correctly

@pes10k pes10k added webcompat/not-shields-related Sites are breaking because of something other than Shields. webcompat/shields Shields is breaking a website. labels May 12, 2020
@pes10k
Copy link
Contributor Author

pes10k commented May 12, 2020

This is related to #8514

@iefremov
Copy link
Contributor

Should be resolved in brave/brave-core#5533

@LaurenWags
Copy link
Member

@pes10k @iefremov @bridiver could we get a test plan for this one please? If no manual QA is required, let's change the label to QA/No. Thanks! cc @rebron @kjozwiak

@pes10k
Copy link
Contributor Author

pes10k commented Jul 30, 2020

Hi @LaurenWags , i just added tests for this at https://dev-pages.brave.software/storage/exceptions.html. I think the text there explains the cases, but as long as the "received" column matches the "expected" column when shields are up, then it should be golden

@LaurenWags
Copy link
Member

LaurenWags commented Jul 30, 2020

Thanks @pes10k!

Verified passed with

Brave | 1.12.105 Chromium: 84.0.4147.105 (Official Build) (64-bit)
-- | --
Revision | a6b12dfad6663f13a7e16e9a42a6a4975374096b-refs/branch-heads/4147@{#943}
OS | macOS Version 10.14.6 (Build 18G3020)

Screen Shot 2020-07-30 at 1 44 54 PM


Verification passed on


Brave | 1.12.105 Chromium: 84.0.4147.105 (Official Build) (64-bit)
-- | --
Revision | a6b12dfad6663f13a7e16e9a42a6a4975374096b-refs/branch-heads/4147@{#943}
OS | Windows 10 OS Version 1903 (Build 18362.959)



Verification passed on

Brave 1.12.105 Chromium: 84.0.4147.105 (Official Build) (64-bit)
Revision a6b12dfad6663f13a7e16e9a42a6a4975374096b-refs/branch-heads/4147@{#943}
OS Ubuntu 18.04 LTS

@bbondy bbondy added OS/Desktop OS/Android Fixes related to Android browser functionality labels Aug 7, 2020
@rebron rebron changed the title Don't throw when accessing local,sessionStorage in 3p frame Don't throw an exception when accessing local,sessionStorage in 3p frame Aug 10, 2020
@srirambv
Copy link
Contributor

Verification passed on Samsung Tab A with Android 10 running 1.12.111 x64 build


Verification passed on OnePlus 6T with Android 10 running 1.12.111 x64 build

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

Successfully merging a pull request may close this issue.

8 participants