Detect if access to localStorage is forbidden by the user's browser #55080
Conversation
|
Some changes occurred in HTML/CSS. |
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @GuillaumeGomez (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
Note: I'm not sure how to actually test this change, so I haven't done that yet. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
If the user's cookie/persistent storage setting forbid access to localStorage, catch the exception and abort the access. Currently, attempting to use the expand/contract links at the top of the page for structs/consts/etc. fails due to an unhandled error while accessing localStorage, if such access is forbidden, as the exception from the failed access propagates all the way out, interrupting the expand/contract. Instead, I would like to degrade gracefully; the access won't happen (the collapse/expand state won't get persisted) but the actual expanding/contracting of the item will go on to succeed. Fixes rust-lang#55079
thanatos
force-pushed
the
fix-localstorage-crash
branch
from
76d2d57 to
d4e2dca
Compare
|
Thanks for the PR! Please update the "getCurrentValue" function (just below the one you updated) too. |
…tValue() 1. Extract the tests for whether or not we have workable localStorage out into a helper method, so it can be more easily reused 2. Use it in getCurrentValue() too, for the same reasons, as suggested in code review
|
Given the way LMK if there's anything else needed; also, feel free to squash/not squash however the rust-lang folks like to do. I ran var resourcesSuffix="";just prior to the copyright banner; I have no idea where that comes from, so I left it for my test, only replacing the stuff below the copyright banner. If there's some more rigorous test I could perform, LMK, and I'll do that too.) |
| return localStorage[name]; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| function usableLocalStorage() { |
There was a problem hiding this comment.
Can move this function above the two others please? It's just me being careful but better be safe than sorry. :)
There was a problem hiding this comment.
Done.
The order shouldn't matter; in JS, function declarations are "hoisted" to the top of the file (in the case that they're at the top-level, and not contained in another function), so they appear to happen prior to code actually running. (Function expressions are not, and the MDN article notes that.)
There was a problem hiding this comment.
Try this code then:
foo();
function foo() {
console.log('hello');
}
|
Thanks! @bors: r+ rollup |
|
📌 Commit cbe98ec has been approved by |
bors
added
S-waiting-on-bors
…GuillaumeGomez Detect if access to localStorage is forbidden by the user's browser If the user's cookie/persistent storage setting forbid access to `localStorage`, catch the exception and abort the access. Currently, attempting to use the expand/contract links at the top of the page for structs/consts/etc. fails due to an unhandled error while accessing `localStorage`, if such access is forbidden, as the exception from the failed access propagates all the way out, interrupting the expand/contract. Instead, I would like to degrade gracefully; the access won't happen (the collapse/expand state won't get persisted) but the actual expanding/contracting of the item will go on to succeed. Fixes rust-lang#55079
Rollup of 18 pull requests Successful merges: - #54646 (improve documentation on std::thread::sleep) - #54933 (Cleanup the rest of codegen_llvm) - #54964 (Run both lldb and gdb tests) - #55016 (Deduplicate some code and compile-time values around vtables) - #55031 (Improve verify_llvm_ir config option) - #55050 (doc std::fmt: the Python inspiration is already mentioned in precedin…) - #55077 (rustdoc: Use dyn keyword when rendering dynamic traits) - #55080 (Detect if access to localStorage is forbidden by the user's browser) - #55090 (regression test for move out of borrow via pattern) - #55102 (resolve: Do not skip extern prelude during speculative resolution) - #55104 (Add test for #34229) - #55111 ([Rustc Book] Explain --cfg's arguments) - #55122 (Cleanup mir/borrowck) - #55127 (Remove HybridBitSet::dummy) - #55128 (Fix LLVMRustInlineAsmVerify return type mismatch) - #55142 (miri: layout should not affect CTFE checks (outside of validation)) - #55151 (Cleanup nll) - #55161 ([librustdoc] Disable spellcheck for search field)
|
I think it'd be safer and more future-proof to wrap For example, if browsers implemented read-only |
|
The write can only be performed on the setting page, therefore it's not really an issue if it doesn't work. The setting will just not get updated. |
If the user's cookie/persistent storage setting forbid access to
localStorage, catch the exception and abort the access.Currently, attempting to use the expand/contract links at the top of the page for structs/consts/etc. fails due to an unhandled error while accessing
localStorage, if such access is forbidden, as the exception from the failed access propagates all the way out, interrupting the expand/contract. Instead, I would like to degrade gracefully; the access won't happen (the collapse/expand state won't get persisted) but the actual expanding/contracting of the item will go on to succeed.Fixes #55079