element-android icon indicating copy to clipboard operation
element-android copied to clipboard

Custom widget does not work in v1.5.20

Open korneyda opened this issue 2 years ago • 11 comments

Steps to reproduce

  1. Where are you starting? What can you see? Add widget in Element Desktop with /addwidget YOU_URL Example: /addwidget https://www.google.com
  2. Widget open successfully in element desktop
  3. But this widget does not open in element android

https://user-images.githubusercontent.com/13951456/214316111-b69f6b45-a4df-4614-97be-623d40da908f.mp4

Outcome

What did you expect?

Custom widget should open

What happened instead?

Not success

Your phone model

No response

Operating system version

Android 12

Application version and app store

Element v1.5.20

Homeserver

No response

Will you send logs?

No

Are you willing to provide a PR?

Yes

korneyda avatar Jan 24 '23 14:01 korneyda

Can confirm this is still an issue with current android element release. Custom widgets are essentially broken loading forever unless you open in external browser.

spiderfudge avatar Feb 06 '23 23:02 spiderfudge

I can confirm. Widgets and stickers do not work on version 1.5.20+ Looks just like the video above. Last working version 1.5.18. Tested on 2 different devices, Android 12 & 13.

747iajc1k avatar Feb 13 '23 07:02 747iajc1k

Also confirming on 1.5.24. Widgets that used to work now spin forever.

Interestingly, an etherpad widget added by the integration manager works, but the existing etherpads I host, dont work.

nik0kin avatar Feb 18 '23 16:02 nik0kin

Confirming, version 1.5.25 does not load custom widgets. This is quite important for us, since we are using OpenID to authenticate users and it doesn't work through an external website.

murlock1000 avatar Feb 24 '23 11:02 murlock1000

FWIW, this fixes it (at least for stickers, haven't tested others): https://github.com/SchildiChat/SchildiChat-android/commit/c47b9ee64d1d760703ade0e5612dfb2c817fd58e but I haven't PR'ed it since it doesn't look like a proper fix to me, haven't really investigated why it doesn't work.

SpiritCroc avatar Feb 24 '23 11:02 SpiritCroc

This seems to fix it: SadoP@d268d2f

Disclaimer: I'm not an android developer, I know just enough to poke around in software. If someone who knows more about this than I do can confirm that this doesnt break something else, I'll open a PR.

SadoP avatar Feb 24 '23 23:02 SadoP

@ganfra could this be looked into? Seems like a simple fix, but maybe the person who's familiar with that part of the codebase knows more.

murlock1000 avatar Feb 28 '23 12:02 murlock1000

I figured out the core of this issue. Basically we have a race condition caused by this new observeViewEvents function: https://github.com/SchildiChat/SchildiChat-android/commit/9c79d234440310bf41e4964c78dc48e8bbb89c15

observeViewEvents only starts listening when the thing is resumed, but also we call loadFormattedUrl immediately https://github.com/vector-im/element-android/blob/ea9874adca787bedabc93e0e1ef3725c0f1463e2/vector/src/main/java/im/vector/app/features/widgets/WidgetFragment.kt https://github.com/vector-im/element-android/blob/75de805417ffea6cd2b1647e098d1d32f8e3f17b/vector/src/main/java/im/vector/app/features/widgets/WidgetViewModel.kt#L146

This means that the onUrlFormatted event is emitted immediately, even before the observeViewEvents reaches the appropriate lifecycle.

So why does it work for the "standard" widgets but not custom widgets? Because here for whitelisted urls they make a network call to fetch the scalar token. This means that the format url takes tens of milliseconds, enough so that the lifecycle has advanced far enough that the events are actually being listened to. So by virtue of making this extra slow api call, the element devs have accidentally avoided this race condition, but only for their own whitelisted urls. https://github.com/vector-im/element-android/blob/884525bef0b35bdc1387271e87172bff5e890baa/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/widgets/DefaultWidgetURLFormatter.kt#L71

It's clear that this isn't logically correct code, and it seems that the justification for making the change is quite flimsy.

We might also be safer with listening viewEvents flow from ViewModel, to avoid refreshing UI if the app is in background:

This was to prevent potential ui updates while the app is in the background, but there is no reason to believe that this is actually semantically correct, especially in our situation where it doesn't work correctly. There are also reports of this causing issues in #7876 though I can't vouch for that. https://github.com/vector-im/element-android/issues/7722

It seems clear to me that #7724 should be reverted in part and its change to fix deprecation should be split off from the major functional and logical change that is causing this entire feature not to work at all. Of course it works for their own whitelisted widgets so they couldn't give a darn about the rest of us, maybe if we beg hard enough they will accept a patch on this.

AndrewRyanChama avatar Mar 09 '23 06:03 AndrewRyanChama

Any updates on this @ganfra ?

murlock1000 avatar Mar 27 '23 23:03 murlock1000

My stickerpicker widget does not work either.

K1D77A avatar Apr 10 '23 16:04 K1D77A

Any updates on this? I am using 1.6.10 and it still fails to open sticker packs.

varisht-tathya avatar Feb 14 '24 06:02 varisht-tathya