Remove chat input from session storage to prevent passing it into messagebox in another game #16587
Conversation
…aving it between games.
|
Hi, thanks for the PR. This change was suggested in #14635 but see the comment for why it was declined at the time. |
Hello, thank you for your feedback. The difference between my PR and #14635 is that solution given there would remove chat.input from session storage when the game ends (which wouldn't be okay in a situation where player and their opponent would want to chat after the game finishes), while my fix clears chat.input on new chatbox initalization instead, which I thought should be alright. |
|
If you delete the chat input storage on every page load, then what use is left for that storage? It seems to defeat the entire point, or I missed something. |
You're right - that would achieve the same result. I can try to fix the PR, but I'd like to understand - what's the exact purpose of chat.input in session storage? How should the end-result of using it look like? I can guess that it should save and load chat.input between games against same users (any games or possibly rematches-only), but I'm not sure. I'd like to also know what's the expected behaviour while:
Thanks in advance. |
|
I don't quite remember why it was added. But I can guess it helps when you play a tournament. The page can redirect to your new game, while you're typing in the chat. With the storage, your text isn't lost, and can even post it on the game page, since it shares the tournament chat. |
…in tournaments and resolve unexpected input persistence in unrelated games
I fixed the PR, now it should work with desired effect.I added 2 keys to session storage, one tracking latest competition (arena/swiss) user took part in. The other one tracks the user's game that was just initiated. Rematch caseRematchMessageSave.mp4Arena tournament caseArenaMessageSave.mp4 |
| const competitionRegex = new RegExp( | ||
| `^https:\\/\\/lichess\\.org\\/((tournament)|(swiss))\\/${tempStorage.get('competition.id')}\\/?$`, | ||
| ); | ||
| const newGameRegex = new RegExp(`^https:\\/\\/lichess\\.org\\/(${tempStorage.get('newGame.id')})\\/?$`); |
There was a problem hiding this comment.
ui/chat should know about chatting. It should not know about:
- lichess.org
- https
- tournaments and swiss
- URL formats for competitions and games
- location.href
And it should not assume that other parts of the site will write to competition.id and newGame.id.
If this change requires adding that kind of hacks, then I say it's not worth it.
Fixes the issue #14602
video.mp4