-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Refactor Web storage #5560
Refactor Web storage #5560
Conversation
93ac38d
to
61ec86c
Compare
This can be used by APIs that share keying logic with storage, such as BroadcastChannel and shared workers. Also add and export create a storage shelf so HTML can more cleanly clone sessionStorage data as part of whatwg/html#5560. Closes #92.
"legacy-clone a browsing session storage shed" can be used by HTML to define creation of auxiliary browsing contexts, as part of whatwg/html#5560. "obtain a storage key" can be used by APIs that share keying logic with storage, such as BroadcastChannel and shared workers. See whatwg/html#3054. It's potentially also useful for Indexed DB as discussed in w3c/IndexedDB#334. Closes #92.
This now depends on whatwg/storage#93. I'm happy with the normative aspects, except perhaps that there's not a clean way to encapsulate that modifying the map can fail and that it seems we only address that when setting. Though the latter is an existing issue. I haven't really thought long about the non-normative parts. I think a fair bit could probably move to the Storage Standard, but I'm not sure to what extent. |
cc @whatwg/storage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "lost" text consists of the following:
Each top-level browsing context has a unique set of session storage areas, one for each origin.
Sounds like this is moving to "browsing session" in some way, and generally handled by Storage? I'm OK removing this, as the domintro makes it vaguely clear that session storage is per-origin.
User agents should not expire data from a browsing context's session storage areas, but may do so when the user requests that such data be deleted, or when the UA detects that it has limited storage space, or for security reasons. User agents should always avoid deleting data while a script that could access that data is running. When a top-level browsing context is destroyed (and therefore permanently inaccessible to the user) the data stored in its session storage areas can be discarded with it, as the API described in this specification provides no way for that data to ever be subsequently retrieved.
User agents should expire data from the local storage areas only for security reasons or when requested to do so by the user. User agents should always avoid deleting data while a script that could access that data is running.
These should be moved to Storage somewhere, and maybe linked to from here.
The lifetime of a browsing context can be unrelated to the lifetime of the actual user agent process itself, as the user agent can support resuming sessions after a restart.
This seems important for our eventual definition of browsing session, hrm.
In the case of an iframe being moved to another Document, its nested browsing context is destroyed and a new one created.
This seems overly-detailed and I'm fine with it disappearing.
The localStorage attribute provides access to shared state. This specification does not define the interaction with other browsing contexts in a multiprocess user agent, and authors are encouraged to assume that there is no locking mechanism. A site could, for instance, try to read the value of a key, increment its value, then write it back out, using the new value as a unique identifier for the session; if the site does this twice in two different browser windows at the same time, it might end up using the same "unique" identifier for both sessions, with potentially disastrous effects.
I think this is worth keeping (and generalizing to include sessionStorage?) in some form. It's unique to localStorage and its sync nature, as opposed to IDB or similar.
Also worth noting, the whole "Disk space", "Privacy", and "Security" sections might be best moved to Storage. That could be done in a separate PR though.
source
Outdated
keys were last added to the storage area.</p> | ||
<dt><dfn data-x="concept-Storage-type">type</dfn> | ||
<dd>"<code data-x="">local</code>" or "<code data-x="">session</code>". | ||
</dl> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I'd just inline these into the preceding <p>
since they're so short and simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the type between parenthesis or something? I quite like using dl for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking
has an associated map, which is a story proxy map, and type, which is either "local" or "session".
I think dl makes sense usually but it takes up a lot of vertical space for simple things like this, where the dds are only a few words. It's more useful when the dds are long explanations.
Maybe it would be better if we had a dl class="brief" which made each item take a single line instead of two lines.
Useful for whatwg/html#5560.
Useful for whatwg/html#5560.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting solid. Optional: I think the order of the whole section is a bit strange, with interface/getter/getter/interface. My preferred ordering would be localStorage/sessionStorage/Storage/StorageEvent, although putting localStorage before sessionStorage would also require some changing of the introduction, so sessionStorage/localStorage/Storage/StorageEvent is also fine.
source
Outdated
keys were last added to the storage area.</p> | ||
<dt><dfn data-x="concept-Storage-type">type</dfn> | ||
<dd>"<code data-x="">local</code>" or "<code data-x="">session</code>". | ||
</dl> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking
has an associated map, which is a story proxy map, and type, which is either "local" or "session".
I think dl makes sense usually but it takes up a lot of vertical space for simple things like this, where the dds are only a few words. It's more useful when the dds are long explanations.
Maybe it would be better if we had a dl class="brief" which made each item take a single line instead of two lines.
Having a central point to acknowledge its non-existence will hopefully provide some clarity for #5560 and other changes.
Having a central point to acknowledge its non-existence will hopefully provide some clarity for #5560 and other changes.
b36a62f
to
319789d
Compare
whatwg/storage#93 and whatwg/storage#97 tackle the Storage side of this. I suspect moving the Privacy/Security sections is best done as a follow-up as it is getting rather unwieldy already. |
"legacy-clone a browsing session storage shed" can be used by HTML to define creation of auxiliary browsing contexts, as part of whatwg/html#5560. "obtain a storage key" can be used by APIs that share keying logic with storage, such as BroadcastChannel and shared workers. See whatwg/html#3054. It's potentially also useful for Indexed DB as discussed in w3c/IndexedDB#334. Also helps a bit with #95 by reorganizing and adding some more detail to how a user agent is supposed to manage storage. Closes #92.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! A couple nits about non-normative sections.
source
Outdated
<code>Document</code>'s <span>origin</span> is an <span data-x="concept-origin-opaque">opaque | ||
origin</span> or if the request violates a policy decision (e.g. if the user agent is | ||
configured to not allow the page to persist data).</p> | ||
<p>Returns the <code>Storage</code> object associated with that <var>window</var>'s origin's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe update to something more correct than "origin's session storage area" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to keep area here for now until we move the Privacy/Security sections.
This seems editorially reasonable but I continue to be pretty discontented with the removal of all the helpful text and replacing it by less helpful things in Storage, or in some cases by nothing. So I'd rather resolve whatwg/storage#95 first. |
9db9276
to
d4855e3
Compare
This makes use of the new primitives in the Storage Standard.
Closes #3210, closes #3283, closes #4650, and closes #5463.
Actually obtaining a storage bottle and dealing with storage bottle cloning to close #5498 still needs to be written down.
Some (removed) notes should maybe also move into Storage or elsewhere within HTML.
(See WHATWG Working Mode: Changes for more details.)
/browsers.html ( diff )
/index.html ( diff )
/infrastructure.html ( diff )
/references.html ( diff )
/webstorage.html ( diff )