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

WIP: Add support for XEP-0156 #1170

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@linkmauve
Contributor

linkmauve commented Aug 19, 2018

It is disabled by default until more testing has been done, set
use_xep_0156 to true for now.

There is also the very big problem that the Strophe.Connection is
created before the user has had the time to enter their JID, which is
why we currently hardcode the domain to be resolved (this MUST be fixed
before this can be merged). I haven’t been able to figure out where to
move this Connection creation, which must be after the JID is known but
before the actual connection is attempted.

There is also a bit of code duplication in it, this will be fixed in a
future version of this commit.

Fixes #129.
Fixes #1109.

@linkmauve linkmauve force-pushed the linkmauve:xep-0156 branch from 0b11ce2 to e7350e6 Aug 19, 2018

@jcbrand

This comment has been minimized.

Member

jcbrand commented Aug 20, 2018

@linkmauve The login code is fairly complex because there are multiple different ways a user could log in.

They could log in manually with the login form or automatically via XHR-fetched credentials, config credentials passed in to converse.initialize or via BOSH prebind.

Also, the different login types happen at different times within startup/setup of the client.
Manual login happens after converse.initialize has completed, automatic login happens before it has completed.

Due to this there is no one place where login can happen. It can happen in converse-core (automatic) or in converse-controlbox (manual).

Manuel login happens in the authenticate method of the LoginPanel view in converse-controlbox.js

Automatic login happens in various places in converse-core.js, such as (but not only) the autoLogin method.

In all these methods, it's assumed that converse.connection exists.

Modern JavaScript has a getter syntax which allows you to call a method as if it was an attribute, similar to Python's @property decorator.

I think we should rewrite the initConnection method to be a getter named connection instead.

Something like:

const converse = {
    get connection () {
        // code comes here
    }
}

Then, whenever _converse.connection is called, the connection will be instantiated if it doesn't exist already.

That might not be enough yet, because the connection might still be instantiated in code at times when we don't have a JID, but I think it's a good first step and those problematic cases will then have to be refactored somehow.

linkmauve added some commits Jun 17, 2018

WIP: Add support for XEP-0156.
It is disabled by default until more testing has been done, set
use_xep_0156 to true for now.

There is also the very big problem that the `Strophe.Connection` is
created before the user has had the time to enter their JID, which is
why we currently hardcode the domain to be resolved (this MUST be fixed
before this can be merged).  I haven’t been able to figure out where to
move this Connection creation, which must be after the JID is known but
before the actual connection is attempted.

There is also a bit of code duplication in it, this will be fixed in a
future version of this commit.

Fixes #129.
Fixes #1109.

@linkmauve linkmauve force-pushed the linkmauve:xep-0156 branch from e7350e6 to 88b985d Oct 10, 2018

@jcbrand jcbrand force-pushed the conversejs:master branch 2 times, most recently from 81498d6 to 6bab16d Oct 29, 2018

@jcbrand jcbrand closed this Nov 20, 2018

@jcbrand

This comment has been minimized.

Member

jcbrand commented Nov 20, 2018

Closed in favour of #1340

@linkmauve linkmauve deleted the linkmauve:xep-0156 branch Nov 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment