element-hq / element-web

A glossy Matrix collaboration client for the web.
https://element.io
Apache License 2.0
10.99k stars 1.95k forks source link

We allow room avatars to be set to SVGs, but they then fail silently #2581

Open come-maiz opened 7 years ago

come-maiz commented 7 years ago

I'm trying to change the avatar in my room to this one: http://people.ubuntu.com/~elopio/testfiles/jaquerEspeis.svg

I go to the settings of the room and select the file, and it seems to load properly: ekrankopio de 2016-11-14 22-51-38 However, after I save it, the default icon remains: ekrankopio de 2016-11-14 23-01-24

I'm using ubuntu zesty, firefox 49.0 and https://riot.im/app/

richvdh commented 7 years ago

Yeah; SVGs don't work as room avatars. Updated the subject accordingly.

richvdh commented 7 years ago

The problem is that the avatar is set successfully, but when vector attempts to show it in the room, it tries to fetch a thumbnail, which fails.

Not sure what the right solution is here. Should we display the full image if fetching the avatar fails (as we appear to do in the Room Settings on first update)?

ara4n commented 7 years ago

We should do clientside-thumbnailing before uploading, i think, given we support this now for e2e attachments.

ara4n commented 7 years ago

see also #1833 and #1962

linuxtim commented 5 years ago

When set they kinda work (clicking on the default room icon shows the SVG), but most of the UI ignores them and uses a default circular+text icon.

Currently once you set the avatar to be an SVG, you don't seem to be able to change it to anything else using RIOT (I haven't tried with anything else), which is definitely a problem!

I'm testing with https://github.com/rust-embedded/wg/blob/master/assets/logo/ewg-logo-blue-white-on-transparent.svg

turt2live commented 5 years ago

It should be possible to change them after setting, although the quick access might not be there anymore. The gear icon in the top right should bring you to room settings where you can click the square avatar and change it.

linuxtim commented 5 years ago

Yes, the UI appears to offer that (as you described), but the change from svg to png doesn't seem to stick after the dialogue is closed (tried on both release and dev versions of RIOT).

  1. Set room avatar to https://github.com/rust-embedded/wg/blob/master/assets/logo/ewg-logo-blue-white-on-transparent.svg
  2. Set room avatar to https://github.com/rust-embedded/wg/blob/master/assets/logo/ewg-logo-blue-white-on-transparent.png

Browser: Firefox 67.0.4

turt2live commented 5 years ago

hmm, does the save button become enabled? It doesn't apply immediately to give people an opportunity to say "oh no, that's the wrong avatar".

linuxtim commented 5 years ago

Can't see any save button.

turt2live commented 5 years ago

That's more worrying. It should be here: image

linuxtim commented 5 years ago

OK, got it, sorry, because the avatar is off to the right, I didn't associate it with that save button, and assumed it would just immediately take effect (because no save button appeared to be associated with it).

The appearance is of a distinct element which is off in a right-hand side-bar I think.

So, the desired change worked, sorry for the noise on this bug, but probably the non-obviousness of the save button being associated with that preview is a UX bug.

turt2live commented 5 years ago

Indeed, I've opened https://github.com/vector-im/riot-web/issues/10170 to track this.

ara4n commented 4 years ago

as per https://github.com/vector-im/riot-web/issues/2008, we could just blat SVGs through https://github.com/cure53/DOMPurify to allow them in avatars.

LecrisUT commented 3 weeks ago

Hello, we just got hit by this any news on this, and it seems to affect the other Element clients as well. Over at #2008, @revolunet was offering help, any luck on that front?