[stable17] Do not immediately join the conversation in public share pages #2347
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danxuliu
added
the
feature: talk-sidebar ⬅️
|
Is it not possible to move this all into one step? Because splitting both requests into get token / create token will still require to fire 2 requests later on. So instead of making it more complicated, just delay "everything" until the button was pressed? |
Before as soon as the public share page loaded the user or guest automatically joined the conversation. Now the conversation must be explicitly joined by clicking a button in the Talk sidebar. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Until now the room token was got just after loading the page; getting the room token creates the room if it does not exist yet, so even if a user does not explicitly join the room the room could be created, which would reveal when a receiver of a link share opened the link. To prevent that now the sidebar is still shown as soon as the page loads, but getting the room token is deferred until it is actually needed (that is, when the user explicitly joins the room). Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
force-pushed
the
do-not-immediately-join-the-conversation-in-public-share-pages
branch
from
9a9444b
to
e85ee2e
Compare
It was not possible before, because the Talk sidebar was loaded for every share and then, depending on whether it was possible to get a room token for that share or not the sidebar was shown or kept hidden. But now the Talk sidebar is no longer loaded for folders, so it should be safe to do it in one step. |
nickvergessen
approved these changes
Nov 6, 2019
nickvergessen
deleted the
do-not-immediately-join-the-conversation-in-public-share-pages
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before as soon as the public share page loaded the user or guest automatically joined the conversation. Now the conversation must be explicitly joined by clicking a button in the Talk sidebar.
However, the current approach still has a problem: if the conversation was not created yet as soon as the public share page is loaded the conversation is created, even if the user does not join it. When the conversation is created a system message is shown, so this could still be used for "stalking" (knowing when the recipient of a link share opened it for the first time).
This could be fixed by:
Not adding the "XXX created the conversation" system message for file rooms (which I am not sure if it is very useful/needed in that case anyway).
Splitting getting the token for a file room and creating the file room. Currently, when the room token is got and the room does not exist yet the room is automatically created. To fix the issue the UI should first try to get the room token and, if it does not exist yet, explicitly create it once the user wants to join the room.
This would involve changes to PublicShareController, which was merged with FilesIntegrationController for Talk 8, so it would be better to backport Free FilesController #2189 and Fix minor php issues #2245 to stable17 to avoid working twice when porting this to master.
@nickvergessen Thoughts? I would go with the first option, simply not adding the conversation created system message for file rooms.
Fix #2382