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

There is no spec-compliant way to remove remove/clear a user avatar #378

Open
dkanada opened this issue Sep 17, 2018 · 16 comments
Open

There is no spec-compliant way to remove remove/clear a user avatar #378

dkanada opened this issue Sep 17, 2018 · 16 comments
Labels
A-Client-Server Issues affecting the CS API enhancement A suggestion for a relatively simple improvement to the protocol

Comments

@dkanada
Copy link

dkanada commented Sep 17, 2018

I think it would be useful to be able to set an avatar to null again with an event, but I don't see the option anywhere in the android client. Is it just not implemented yet on android or is it not in the spec yet?

@turt2live
Copy link
Member

Looks like it doesn't say it, but a PUT with avatar_url: null will get the job done. I've updated the title of this issue to represent the omission.

@turt2live turt2live added spec-omission implemented but not currently specified A-Client-Server Issues affecting the CS API labels Sep 17, 2018
@turt2live turt2live changed the title Does the spec support removing a user or room avatar? Clarify that making a PUT for avatar_url: null removes/clears the avatar Sep 17, 2018
@dkanada
Copy link
Author

dkanada commented Sep 17, 2018

I can't seem to find the API call to set a room avatar, does it work the same way? Also, would a null value be an empty string for the property or an empty json call? Haven't worked with the API on the android client yet, just want to make sure I understand correctly.

@bmarty
Copy link

bmarty commented Sep 17, 2018

@dkanada not sure it will work on Android, as null serialization is not enabled by default on the Rest client

@dkanada
Copy link
Author

dkanada commented Sep 17, 2018

Is there any chance to get this working then? Since only certain API calls allow a null value, it could just be enabled where necessary, like setting newUrl in this method to accept a null value. Could also add an endpoint for deleting a user avatar.

/_matrix/client/r0/profile/{userId}/avatar_url/remove

@turt2live
Copy link
Member

The API endpoint would be https://matrix.org/docs/spec/client_server/r0.4.0.html#put-matrix-client-r0-profile-userid-avatar-url where the avatar_url is specified as null, however that would be done. Issues/questions specific to riot-android should be directed at riot-android though, despite there being lack of documentation in the spec for this area.

@turt2live
Copy link
Member

Actually, it looks like the spec allows one to post an empty object and achieve the same goal, it just doesn't say that. Changing this to a clarification and not an omission.

@turt2live turt2live added clarification An area where the expected behaviour is understood, but the spec could do with being more explicit and removed spec-omission implemented but not currently specified labels Sep 17, 2018
@bmarty
Copy link

bmarty commented Sep 17, 2018

@dkanada ,
Not without a specific change. As you can see, the constructor of ProfileRestClient call the super constructor with false for the last parameter (withNullSerialization)

@bmarty
Copy link

bmarty commented Sep 17, 2018

I think the best solution for a full Rest server should be to use the DELETE method for this API:
DELETE /_matrix/client/r0/profile/{userId}/avatar_url

@dkanada
Copy link
Author

dkanada commented Sep 18, 2018

I didn't see too many delete calls in the client server API though, it seemed like you guys were trying to avoid them. Does the redact feature use delete for events?

Regardless, would an empty object work for this purpose, or maybe just an empty string for the avatar_url field? It hasn't been revisited yet but I would also like to add group icon removal as well, which would probably be a separate change.

@turt2live
Copy link
Member

It's not that the spec tries to avoid DELETE, it's just that there's not a lot of use for it. Redacting doesn't use DELETE because redacting doesn't delete things, it redacts them which is an action. Events in matrix cannot be truly deleted.

An empty object would work as per my last comment. An empty string probably wouldn't have the effect you're looking for. Groups aren't in the spec yet, but I imagine they'll have similar semantics.

@turt2live
Copy link
Member

So it turns out that the avatar_url is required, and we've decided that null isn't necessarily valid (https://github.com/matrix-org/matrix-doc/issues/2011). I guess an empty string would be the current way to do things, or possibly introducing a DELETE endpoint - I'd personally be in favour of a DELETE endpoint.

@KitsuneRal
Copy link
Member

In whichever case, both user and room avatar cases should be addressed; and since in case of rooms an avatar is defined by a state event, introducing a DELETE endpoint would probably lead us to the long standing question of ability to remove other state events as well.

@dkanada
Copy link
Author

dkanada commented Jul 8, 2019

I just found out group names (which I think are state events) can be removed so I agree that something should definitely be added to the spec for avatars as well.

@dkanada
Copy link
Author

dkanada commented Jan 12, 2020

@turt2live any update on this one?

@jaller94
Copy link
Contributor

Element Web resets avatars by sending an empty string.

@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Mar 1, 2022
@bmarty
Copy link

bmarty commented Mar 10, 2022

Related: element-hq/element-android#4254

@richvdh richvdh changed the title Clarify that making a PUT for avatar_url: null removes/clears the avatar There is no spec-compliant way to remove remove/clear a user avatar Nov 15, 2022
@richvdh richvdh added enhancement A suggestion for a relatively simple improvement to the protocol and removed clarification An area where the expected behaviour is understood, but the spec could do with being more explicit labels Nov 15, 2022
@dkanada dkanada closed this as completed Mar 5, 2023
@richvdh richvdh reopened this Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Client-Server Issues affecting the CS API enhancement A suggestion for a relatively simple improvement to the protocol
Projects
None yet
Development

No branches or pull requests

6 participants