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

Editorial: Make pageshow.persisted non-normative refer to salvageable… #6558

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

nyaxt
Copy link
Member

@nyaxt nyaxt commented Apr 6, 2021

Editorial change - skipping the checklist template.

This is an editorial change which attempts to remove the unsalvageable normative text which I was confused when I was reading through it. We now have the #concept-document-salvageable, so this PR attempts to replace the enumerated conditions with its reference.

Open Question:

  • The removed list contained "Active WebSocket Objects" condition. I couldn't find a normative step that "sets salvageable state to false". Should we also make the change?

/browsing-the-web.html ( diff )

@nyaxt nyaxt requested review from rakina and domfarolino April 6, 2021 01:40
@rakina
Copy link
Member

rakina commented Apr 6, 2021

Hmm, I wonder if we should keep the list. It's kinda nice to have a list of things in the note instead of having to go through the reference. Referring to the "salvageable" concept instead of using "unsalvageable" sounds good though.

The WebSockets bit is specified here, but there's an open issue about it: #1931

@nyaxt
Copy link
Member Author

nyaxt commented Apr 6, 2021

Thanks for the pointer for the WebSockets bit - I missed it.
I'll update the PR to keep the list.

@nyaxt nyaxt force-pushed the persistedNonNormative branch 2 times, most recently from e5d9307 to 2d3f20e Compare April 7, 2021 00:07
@nyaxt
Copy link
Member Author

nyaxt commented Apr 7, 2021

@rakina I addressed your comment. PTAL

Copy link
Member

@rakina rakina left a comment

Choose a reason for hiding this comment

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

Thanks, this looks like a nice editorial change. I have some suggestions to make it even more readable, and probably @domenic needs to review this too (for the write access bit?)

source Show resolved Hide resolved
@nyaxt nyaxt force-pushed the persistedNonNormative branch from 2d3f20e to 3902f60 Compare April 7, 2021 01:32
Copy link
Member

@rakina rakina left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@nyaxt nyaxt requested a review from domenic April 7, 2021 03:44
@nyaxt
Copy link
Member Author

nyaxt commented Apr 7, 2021

Thanks, Rakina!

@domenic Can I have your review?

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Thanks!

@domenic domenic merged commit 1ce839b into whatwg:main Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants