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

Bug Fix: Surface Error for when cookie settings prevent localStorage #21503

Merged
merged 4 commits into from
Jul 7, 2023

Conversation

Monkeychip
Copy link
Contributor

@Monkeychip Monkeychip commented Jun 28, 2023

The new Dismiss License Banner feature uses localStorage. Because the banners are loaded before a user attempts to login, the app was silently failing if a browsers settings did not permit localStorage.

To reproduce:

  1. Under your browsers privacy settings select "block all cookies".
    image
  2. Load the app.

Before the Dismiss license banners work we caught this error on login and surfaced it in the Login container. While the app still didn't work, it didn't silently fail on first load.

This fix shows the DomException error thrown and appends a small clarifying message afterwards. I didn't want to add a lot of custom logic to the error.hbs template for this one scenario, so it's pretty simple.

Note the error that is thrown is readOnly. Here are the MDN docs on the three params returned.

image

@VioletHynes VioletHynes added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 6, 2023
Copy link
Contributor

@zofskeez zofskeez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work and helpful to surface this as an error! I pulled down the branch and verified that the error is displaying on Brave, Chrome, Safari and Firefox.

@Monkeychip Monkeychip merged commit 9c8a742 into main Jul 7, 2023
61 checks passed
@Monkeychip Monkeychip deleted the ui/VAULT-17601/check-local-storage-supported branch July 7, 2023 16:21
Monkeychip added a commit that referenced this pull request Jul 8, 2023
…21503)

* initial fix

* changelog

* clean up

---------

Co-authored-by: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants