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

Fallback for incomplete threads #1925

Open
4 tasks done
Tracked by #3 ...
daniellekirkwood opened this issue Nov 15, 2021 · 22 comments
Open
4 tasks done
Tracked by #3 ...

Fallback for incomplete threads #1925

daniellekirkwood opened this issue Nov 15, 2021 · 22 comments

Comments

@daniellekirkwood
Copy link
Contributor

daniellekirkwood commented Nov 15, 2021

When a user is invited to a restricted room that already has threads and messages, what should their experience be?

Discussion:

  • Users should be able to see messages that they have permission to see.
  • Users may not be able to see Threads from the Thread Summary on the Timeline (because they do not have access to the Root Message)
  • Users should have access to the Thread from the Thread List view
  • Users should only see a Thread in the Thread List view if they have permissions to see at least one response (if all messages in a Thread are restricted to them due to permissions, it should not appear in the Thread List view)

Needs:

The user has access to the Thread messages (they have access to) from the Thread list view;

  • 1. How does the Thread Summary in the Thread List view look
  • 2. What message do we share with the user when they don't have access to the root message

When the user has clicked to view a thread that they only have rights to see part of;

  • 3. How does the Thread look like (if the user cannot see the Root message, or the messages following...)
  • 4. What message do we share with the user when they are viewing the Thread but have access to view only a few of the most recent messages

Design

Figma: Restricted History Threads.

@daniellekirkwood
Copy link
Contributor Author

@janogarcia Assigning you and tagging X-Needs-Design as you will need to provide designs to help with this use case.

Please add any details that I might have missed following our conversation this afternoon :)

@daniellekirkwood
Copy link
Contributor Author

@clokep & @gsouquet

Please let us know how much granularity we may be able to show in the error message (items 2 & 4). Depending on what the server/client knows will impact how we communicate with the user!

Thanks!

@clokep
Copy link
Contributor

clokep commented Nov 15, 2021

When a user is invited to a restricted room that already has threads and messages, what should their experience be?

This was discussed a bit in chat, but this doesn't have to do with restricted rooms (which manage who can join a room), but with who has access to the history of a room. Technically this concept is referred to as "history visibility" in the spec and has three options:

  1. World readable: anyone can read the events (so this issue doesn't apply).
  2. Shared: any member can read the events (so this issue doesn't apply).
  3. Invited: events are accessible since a member was invited.
  4. Joined: events are accessible once a member joins the room.

For the specific questions above:

What message do we share with the user when they don't have access to the root message

I would recommend re-using language from a quote reply from a message which you do not have access to. It is a similar concept.

What message do we share with the user when they are viewing the Thread but have access to view only a few of the most recent messages

I wonder if it makes sense for it to show up in the thread pane or if they should just be in the timeline? This isn't ideal, but might simplify some of this. It would give different users different views of the room though, which isn't great.

Please let us know how much granularity we may be able to show in the error message (items 2 & 4). Depending on what the server/client knows will impact how we communicate with the user!

When trying to access an event that does not exist or is not visible the server returns a 404 with no information as to why it is not visible. (There's no way to tell if it is a "real" event without permission to be seen or if it is a bogus event ID.) This is done on purpose to prevent privacy leaks. In practice the only time this should happen is if the user doesn't have permission to see the event, so I think a generic error message about an event being inaccessible is probably fine.

Note that the server will currently not respond to requests for the full thread in this case either, as the root message is not visible. I'm not sure if this is good or bad, but it fits the currently specced behavior of how relations work.

@daniellekirkwood daniellekirkwood changed the title Threads: Access to Threads for new joiner in a Restricted Room Threads: Access to Threads for new joiner in a room (Thread visibility) Nov 15, 2021
@daniellekirkwood
Copy link
Contributor Author

The content in this error message in other parts of the app is :

Unable to load event that was replied to, it either does not exist or you do not have permission to view it.

@daniellekirkwood daniellekirkwood self-assigned this Nov 30, 2021
@daniellekirkwood
Copy link
Contributor Author

James' document regarding all Federation concerns: https://docs.google.com/document/d/12LDwoQ2xQDpZBW52bHMw6U7MLg4jcjw5By480QXxpzU/edit?usp=sharing

@novocaine
Copy link

Although federation might cause similar symptoms, this ticket isn't related to federation, its related to history visibility. We should probably keep it that way, and have a separate ticket for dealing with federation issues.

@janogarcia
Copy link

janogarcia commented Dec 8, 2021

@daniellekirkwood @novocaine @gsouquet I put together some mockups for dealing with this specific issue. You can find my notes on the relevant section on Figma:

Restricted History Threads.

I'm copying below a snapshot of those notes for convenience.

As for other particular issues introduced by federation, I agree with James that those should be discussed in a separate issue.

Note: Today's a national holiday in Spain, and then I'll be on vacation the next two days. I won't be able to follow up on this until next week.


Copied from Figma:

Restricted History Threads

Edge case: The user can’t view messages in a thread prior to her join or invite date, due to room permissions.

In the thread list, we’ll show a notice for restricted history threads, telling the user about that limitation. As we can’t fetch the actual content for the thread’s root message, we’ll append the message event ID, as a way to differentiate between restricted history threads. It’s far from optimal, but there’s no other useful data we can rely on.

Any thread that is getting new replies since the user joined or was invited (depending on room’s permissions) should appear in the user’s thread list.

Threads in which the latest reply happened before that point in time shouldn’t be visible to the user, as there wouldn’t be any viewable content for them.

We should also consider — post P0 milestone and if technically doable, and performant — replacing the original root message with the first reply viewable by the user, and somehow communicating that condition properly to the user.

In the thread view, we’ll show a notice before the first message that is viewable to the user.

https://www.figma.com/file/T309ztx0sNyOOK6NKVLHsK/Threads?node-id=2761%3A308969

@janogarcia
Copy link

@daniellekirkwood We'll also need a similar issue to be created for Android and iOS.

Android: Restricted history threads
https://www.figma.com/file/T309ztx0sNyOOK6NKVLHsK/Threads?node-id=2870%3A283832

iOS: Restricted history threads
https://www.figma.com/file/T309ztx0sNyOOK6NKVLHsK/Threads?node-id=2870%3A284410

@daniellekirkwood
Copy link
Contributor Author

daniellekirkwood commented Dec 17, 2021

Notes from Standup

  • The top of the Threads list will only go back to where you joined the room
  • Can only be discovered on the timeline to reveal the Threads - upon a refresh you would load only the last 20 events and probably not persist the whole list
  • There's a couple of things we can do, they might not be easy.

Next steps

  • @gsouquet to look at potential ways to implement a solution for this
  • The decision that needs to be made is how much of a solve should we provide before launch. We should do what we can to solve for now.

Goal

  • Users should know why they might not have access to an entire thread
  • When a user has joined a room, all new Threads messages will be available to them, but previous Thread responses and Root messages will not be available

Other solutions

  • if the user doesn't have access to root message, can we post thread responses to main timeline?

@janogarcia
Copy link

I've made a few improvements to the first iteration.

I've revisited the icons, copy, as well as some design details. The approach/fallback remains unaltered, though.

Also, I've added a mockup for the improved, post-P0 proposal where we replace the original root message with the first message viewable by the user — as long as that can be implemented in a performant way.

Threads – Figma

@novocaine
Copy link

novocaine commented Jan 11, 2022

Note that Element Web doesn't otherwise explain a room's history being hidden on join - see element-hq/element-web#16074

I feel like whatever we do here should be consistent with any improvement made to that?

@daniellekirkwood
Copy link
Contributor Author

Great point - it would be great to apply at least the same content to both instances so users know why it's blank...

@janogarcia
Copy link

janogarcia commented Jan 11, 2022

@daniellekirkwood @novocaine

As discussed today in the Threads standup, multiple reasons can cause a root message to be missing. At least, to my knowledge, these two:

  • Room's history permissions: You joined a room after a thread was created, and then that thread gets new replies that are visible to you.
  • Federation: Your client is getting new replies to a root message that you couldn't fetch because of a federation issue (delayed message, server went offline...).

The thing is that we can't tell between one or the other (or maybe the root message couldn't even exist at all because the event was crafted manually).

So, are we OK prioritizing the most likely scenario — that is, room's history permissions — and show an error message that makes most sense in that case at the expense or not being accurate for the other scenarios (federation, or crafted stuff)?

Or, should we tweak the copy to make it more generic/encompassing?

@t3chguy
Copy link
Member

t3chguy commented Jan 11, 2022

I've only skimmed all the content in this issue, but one thing that may want be considered at the same time is encryption issues, the root thread event may be undecryptable as even though the room's history permissions allow you to download it, you may not have keys if it was sent prior to your join.

@janogarcia
Copy link

@t3chguy Thanks for pointing that out. 👍

@daniellekirkwood
Copy link
Contributor Author

Or, should we tweak the copy to make it more generic/encompassing?

I would rather make it generic for now

@janogarcia
Copy link

janogarcia commented Jan 12, 2022

@daniellekirkwood @novocaine

The more generic messages should now cover all the scenarios that we identified for causing incomplete threads, and it should hopefully be simple enough to implement for P0:

  • Restricted room history: Unavailable messages. It can affect the root message and the thread history up to a specific point in time.
  • Federation: Delayed or permanently unavailable messages. It can affect both the root message or any other message in the timeline. We're only concerned about the root message in this case.
  • Buggy or malicious client-server: Malformed messages. We're only concerned about the root message in this case.

We're not disclosing if the root message actually exists, nor stating the specific reason (cause we can't know it for sure).

@daniellekirkwood What do you think? Also, I'd appreciate a review from you as a native speaker!

Figma

Snapshot

Incomplete Threads

@janogarcia
Copy link

@daniellekirkwood We should consider renaming this issue to something that better reflects its actual/current scope. "Fallback for incomplete threads" or something along the lines.

@daniellekirkwood daniellekirkwood changed the title Threads: Access to Threads for new joiner in a room (Thread visibility) Threads: Access to Threads for new joiner in a room, Thread visibility, & Incomplete threads Jan 19, 2022
@novocaine novocaine changed the title Threads: Access to Threads for new joiner in a room, Thread visibility, & Incomplete threads Fallback for incomplete threads Mar 10, 2022
@t3chguy
Copy link
Member

t3chguy commented Apr 12, 2022

@daniellekirkwood is this done? It is marked as Shipped yet still open

@novocaine
Copy link

@gsouquet where is this at?

@germain-gg
Copy link

I'm not entirely sure why this ended up on the "Shipped" column. As there's no UI built for this, looks like this fell through the cracks, sorry about that.

However the code has already a way to detect that the root message is missing. In that case the thread will have the rootEvent being undefined.
The thread will have to exist in degraded mode as the APIs defined in MSC3440 will not be able to help clients as synapse will return 404 on a query for the root event

@t3chguy
Copy link
Member

t3chguy commented Apr 25, 2022

The thread will have to exist in degraded mode as the APIs defined in MSC3440 will not be able to help clients as synapse will return 404 on a query for the root event

Which also means it won't be shown in the Threads list because the code doesn't know how to handle mixing degraded and API-ful modes. Especially once that list is a paginated serverside list. element-hq/element-web#21877

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

No branches or pull requests

7 participants