Skip to content
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

Candy should be able to reconnect on its own after temporary loss of network connection #202

Closed
lkraav opened this issue Jan 6, 2014 · 19 comments
Assignees
Labels
Milestone

Comments

@lkraav
Copy link
Contributor

lkraav commented Jan 6, 2014

Right now endless "Disconnect" overlay with spinner is displayed, until URL is manually reloaded. I imagine this can be improved?

@mweibel
Copy link
Member

mweibel commented Jan 6, 2014

We certainly could display the login form again. Saving user/pw combination is not a good option IMO (which would enable Candy to reconnect on it's own).. What do you think?

@lkraav
Copy link
Contributor Author

lkraav commented Jan 6, 2014

I implemented XMPP prebinding last night with the help of your PHP class. Now the login form appears better after loss of connection.

I'm using Basic Auth to prefill l/p over HTTPS, with some modifications to index.php.

I think we have two scenarios here for this issue: prebinding and non-prebinding.

Prebinding:

  • automatically reload the page on network resume, get new prebinding session and basically continue like nothing happened

Non-prebinding:

  • display login form more reliably; reloading the page also accomplishes this

@mweibel
Copy link
Member

mweibel commented Jan 13, 2014

Regarding the specific usecase as described in #210, I guess we could help you if we trigger an event (then you could run window.location.reload() by binding to this event). Running window.location.reload() from Candy by default might not be what people want IMHO.

@lkraav
Copy link
Contributor Author

lkraav commented Jan 13, 2014

Yep, callback sounds nice

@mweibel
Copy link
Member

mweibel commented Jan 13, 2014

There's an event candy:core.login triggered already now. It is always triggered when the login window should be displayed. Might it be possible to _ab_use this one? ;)

@mweibel
Copy link
Member

mweibel commented Jan 16, 2014

You said that "disconnected" is shown. Are you sure it's "disconnected" and not "disconnecting"?
If the state changes to "Disconnected", the Login form should be displayed. Meanwhile "disconnecting" it's not displayed.

@mweibel
Copy link
Member

mweibel commented Jan 16, 2014

Nevertheless, what you can do is the following to alter Candy's default behaviour:

$(Candy).on('candy:core.chat.connection', function(args) {
    switch(args.status) {
        case Strophe.Status.DISCONNECTED:
        case Strophe.Status.ERROR:
        case Strophe.Status.CONNFAIL:
            window.location.reload();
    }
}

Would this solve the issue for you?

@lkraav
Copy link
Contributor Author

lkraav commented Jan 16, 2014

Hmm, I actually wrote "Disconnect" with no suffix. I'm pretty sure it was "Disconnecting...", but I'll double-check it later. Right now this works beautifully:

diff --git a/candy.bundle.js b/candy.bundle.js
index f255e10..1083ff4 100644
--- a/candy.bundle.js
+++ b/candy.bundle.js
@@ -2649,8 +2649,7 @@ Candy.View.Observer = (function(self, $) {
                                        break;

                                case Strophe.Status.DISCONNECTED:
-                                       var presetJid = Candy.Core.isAnonymousConnection() ? Strophe.getDomainFromJid(Candy.Core.getUser().getJid()) : null;
-                                       Candy.View.Pane.Chat.Modal.showLoginForm($.i18n._('statusDisconnected'), presetJid);
+                                       location.reload();
                                        break;

                                case Strophe.Status.AUTHFAIL:

@mweibel
Copy link
Member

mweibel commented Jan 16, 2014

Ok, I see.
I guess my fix would not work as good because it would first show the login form & afterwards reload it (flickering a bit). Soo.. question is: how can we do the same behaviour, without you having to alter the source code.
Maybe before showing the login form, send another event and in case this one returns "false", don't display the login form.

Will try that out.

@lkraav
Copy link
Contributor Author

lkraav commented Jan 16, 2014

I think pluginizing this functionality would be reasonable. If there's events I can hook into, I'll take the burdain of pluginizing it.

mweibel added a commit that referenced this issue Jan 16, 2014
Enable users of Candy to specify if/what happens with the View
before the connection status changes using an event
per connection status as defined in Strophe.Status.
@ghost ghost assigned mweibel Jan 16, 2014
@mweibel
Copy link
Member

mweibel commented Jan 16, 2014

How about this (example code how you could achieve it included).

@lkraav
Copy link
Contributor Author

lkraav commented Jan 16, 2014

Looks lovely to me :) thanks a bunch. It's going to take a few days for me to set it up on my instance, if you want to hold off for some testing before merging.

@mweibel
Copy link
Member

mweibel commented Jan 16, 2014

I'm unsure currently whether it should go with the 1.6.0 release or with 1.6.1. @pstadler, what do you think? I think we could add it to the 1.6.0 release, because otherwise we might need to release a new version soon afterwards.

@pstadler
Copy link
Member

Whatever you want. I'd rather release a bit more often as long as the overhead is managable. If you feel comfortable to release this with 1.6 (and I think we should do it) then please merge it in.

@mweibel
Copy link
Member

mweibel commented Jan 17, 2014

Released beta3 with this fix included. Thanks!

@mweibel mweibel closed this as completed Jan 17, 2014
@lkraav
Copy link
Contributor Author

lkraav commented Jan 17, 2014

Thanks a lot.

Side note: I wonder if restoring the "last active tab" state on reconnect deserves a separate issue. Otherwise reconnect throws the user to the mercy of autojoin order.

@mweibel mweibel mentioned this issue Jan 20, 2014
22 tasks
@caherrerapa
Copy link

Hi @lkraav @mweibel sorry for the silly question, i'm trying to use prebinding (when i use the normal Candy.Core.connect method everything works perfectly) and when i use Candy.Core.attach the status is logged, but i'm not sure how i can trigger the display of the Chat Room Pane. What is the proper way to update the view when using attach()?

I debugged and found that when i use Candy.Core.attach the following lines are not called compared to Candy.Core.connect.

if (!Candy.View.Pane.Chat.rooms[args.roomJid]) {
                    Candy.View.Pane.Room.init(args.roomJid, args.roomName);
                    Candy.View.Pane.Room.show(args.roomJid);
                }

@lkraav
Copy link
Contributor Author

lkraav commented Oct 8, 2014

@caherrerapa I really don't have enough know-how to answer your q

@singpolyma
Copy link

What's the reason not to allow storage of credentials in local user RAM? Most clients do this, and it would allow actual reconnection without user interaction. Maybe as a plugin?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants