zom / Zom-Android-XMPP

THIS PROJECT IS NOW CLOSED. WE HAVE MOVED TO A NEW ZOM 2.0 MATRIX CORE. FOLLOW THE LINK!
https://github.com/zom/zom-android-matrix
GNU General Public License v3.0
141 stars 64 forks source link

Auto-answer subscription request if we already have a one-way subscri… #406

Closed N-Pex closed 6 years ago

N-Pex commented 6 years ago

…ption.

Fixes https://github.com/zom/Zom-iOS/issues/506

I don't have the whole picture here, so please have a look @n8fr8. Android does (no longer?) auto answer the subscription request when the other end has approved us and tries to add us back.

n8fr8 commented 6 years ago

Ah, thanks, will take a look. One thing I fixed is that previous I was using Async Stanza listeners in Smack, which apparently doesn't guarantee correct order. This could account for some of the issue we faced with confusing order of subscription handling.

Otherwise, yes, checking at roster load makes sense!

n8fr8 commented 6 years ago

Hmm, OTOH, just because someone has allowed me to see their presence, doesn't mean I want to automatically let them see mine. That is why we have auto approve subscriptions off.

N-Pex commented 6 years ago

Yes, but if they are on your roster you have already added them there, so should be safe.

N-Pex commented 6 years ago

Or else we need to rework the whole flow here, we can't just enter a chat when one part hits "Approve".

n8fr8 commented 6 years ago

I am confused about rosters... lets talk on the dev scrum!

N-Pex commented 6 years ago

Yup, let's talk, I am probably as confused =)

n8fr8 commented 6 years ago

It is technically possible to have someone in your roster, without having a subscription to or from them, as explained in "2.1.2.5. Subscription Attribute" https://xmpp.org/rfcs/rfc6121.html#roster-syntax-items-subscription

However, it is also true that you should be able to send a message to someone regardless of subscription status, so the "Accept" and open chat model should work.

tiffrobo commented 6 years ago

@n8fr8 Have you been able to address this in the next Android build? We are still seeing this issue between iOS (build 119) and Android.

n8fr8 commented 6 years ago

Yes, I reworked the whole, and I think my new code will handle all tihs appropriately now. Closing this pull request, but please test with the build from Friday.