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

Shorten MAX_REPLACE_STATE_FREQUENCY from 1000 to 300 #3402

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

heavywatal
Copy link

debouncedReplaceState() was introduced to avoid warnings from Safari: "history.replaceState() more than 100 times per 30 seconds". But 1000ms interval is way too long for that.

debouncedReplaceState() was introduced to avoid warnings from Safari:
"history.replaceState() more than 100 times per 30 seconds".
But 1000ms interval is way too long for that.
@t-fritsch
Copy link
Collaborator

Thanks for this PR @heavywatal !

Sorry for the novice question, but could you please explain what problem your PR fixes ? Why is the 1000ms delay too long ?

About this 1000ms delay, it was changed in this commit : 2a239ae

before this commit, the delay was at 250ms, I guess it changed for a reason (any input @hakimel ?), and I wonder if switching it back to 300ms as your PR does wouldn't introduce a regression ?

@heavywatal
Copy link
Author

Thank you for the response.

Suppose you have hundreds of slides to show. Press and hold the down/right/space key to proceed very quickly, then stop. If the reveal is configured history = false and hash = true, you have to wait for 1000ms to get the text in the URL bar updated after releasing the key. Until then, you cannot reload the page or copy the URL. Otherwise it will lead you to the old place. This is a bit annoying when editing slides in a hurry.

But you cannot set MAX_REPLACE_STATE_FREQUENCY to zero because Safari complains "history.replaceState() more than 100 times per 30 seconds". As I wrote, The commit you mentioned was made to avoid this warning, but the interval was much longer than required, which is obviously once per 300ms.

Note that this currently affects only when history = false and hash = true.

@hakimel
Copy link
Owner

hakimel commented Nov 20, 2023

This was set to 1000ms to fire after the transition finishes since changing the hash mid-transition had side effects. It caused a stutter in one of the main browsers. It's been a minute so I can't remember the specifics and would need to rerun tests to see if it's still a factor.

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

Successfully merging this pull request may close these issues.

3 participants