element-hq / element-android

A Matrix collaboration client for Android.
https://element.io/
Apache License 2.0
3.31k stars 702 forks source link

m.image height and widget are float, not int #363

Closed anoadragon453 closed 4 years ago

anoadragon453 commented 5 years ago

The spec mentions an m.images content.info.{h,w} must be integers, however when sending a message today my bridge broke complaining that they were floats: https://github.com/tulir/mautrix-whatsapp/issues/80

They should be converted to floats to remain spec compliant.

bmarty commented 5 years ago

Hello @anoadragon453 ,

Are you sure the image was sent from RiotX? I've checked in the model, and we only use Int for all the data in ImageInfo (and since the beginning): https://github.com/vector-im/riotX-android/blob/develop/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/model/message/ImageInfo.kt

Benoit

anoadragon453 commented 5 years ago

Hm, it was sent from Riot X but it could be something on the bridge side? Odd though, since the bridge is written in Golang which is typed as well. I'll try the mautrix-whatsapp side again.

tulir commented 5 years ago

RiotX definitely sends all integers as floats, you can check the event json in the synapse db.

Maybe the problem is the JSON serializer? The Moshi repo had a bunch of int/float mixup issues, although none of them seemed to be about serialization with JsonClass.

ganfra commented 5 years ago

I've just checked the json sent to the server, and all integers are well sent as integers.

{
   "msgtype": "m.image", 
   "body": "00000PORTRAIT_00000_BURST20190716211350956.jpg",
   "info": {
     "mimetype": "image\/jpeg",
     "w": 4032,
     "h": 3024,
    "size": 3839019,
     "rotation": 0,
    "orientation": 0
   },
}

And the data from the sync is identical.

bmarty commented 5 years ago

@anoadragon453 any information about the device where riotx was installed? Model, os version, etc. A user agent would be ideal. Thanks

tulir commented 5 years ago

My Huawei P20 Pro (Android 9.0) at least does it reliably for every image. Just sent one to #riotx:matrix.org.

"View Source" on Riot Web and RiotX converted them back to ints before showing, but old Riot Android view source and the server db definitely show floats

anoadragon453 commented 5 years ago

Pixel XL, Android 9.0

bmarty commented 5 years ago

I think there is something strange in RiotX.

The view source shows:

When sending an image from RiotX:

When sending an image from Riot Android:

bmarty commented 4 years ago

Hi @anoadragon453 can we close this issue?

tulir commented 4 years ago

The issue still seems to be there on build 936

MTRNord commented 4 years ago

This issue happened now too. See the above ruma issue. It seems like if you fetch the data via the /_matrix/client/r0/rooms/:room_id/event/:event_id endpoint it returns floats. But in riot-web they show up as integers. (Maybe in the sync synapse does make them ints?)

tulir commented 4 years ago

(Maybe in the sync synapse does make them ints?)

No, it's riot web showing you different content than what the event actually contains (as I said above: https://github.com/vector-im/riotX-android/issues/363#issuecomment-512128543)

MTRNord commented 4 years ago

@tulir whoops ok. Yeah but this still happens anyway

auscompgeek commented 4 years ago

Note that this means RiotX Element Android cannot send images to v6 rooms.

ganfra commented 4 years ago

Json actually doesn't really know between double or integer, it's just "numbers" The client is responsible for typing then. In RiotX this issue is caused by loosing the information of the content when sending through Workers. It will be fixed in next release, thanks for pointing it :)

diller444 commented 4 years ago

Huawei P Smart Z, Android 10.0

bmarty commented 4 years ago

Should be fixed by #1766

tulir commented 4 years ago

Sending images to v6 rooms still doesn't work on latest develop (G-b3379)

diller444 commented 4 years ago

still the same