Don't require loading of include.js on every pageview?

[skorokithakis]

Is there a way to not require include.js to be loaded on every pageview? I was pretty happy with BrowserID when that script only needed to be loaded on my landing page (for logins), but now it needs to load on every single page load, slowing down my page, even though a very very small percentage of pageviews lead to a log out.

It's also causing problems by apparently thinking that my session is expired and forces a logout, only to be led to the front page and automatically log in again. As you can imagine, this is a dealbreaker for users.

Just not loading it doesn't work, because clicking the "logout" button leaves users on the landing page, where BrowserID promptly logs back in again.

Is there a way to prevent loading the script until the user actually clicks the logout button, or similar?

[Osmose]

So right now, the way we do logout buttons is that we first run navigator.id.logout(), then redirect the user to the logout view so they can log out with your backend.

The problem with not including include.js is that when the user hits the landing page, Persona still thinks they're logged in, and it attempts to be helpful by triggering login automatically (this is not helpful, but the Persona team refused to change this when I asked about it, so we have to live with it). It runs this check as soon as request.id.watch() is called, which the django-browserid JavaScript calls to set up callbacks for responding to login and logout.

So what we really need is a way to logout and always call navigator.id.logout either before the logout request happens, or after it happens but before navigator.id.watch is called.

Alternatively, we can just ignore any auto-login and auto-logout stuff by refusing to respond to anything that wasn't initiated by django_browserid.login and django_browserid.logout. This would break our ability to auto-login users who have just signed up for Persona for the first time, but it would help alleviate the pain we've gotten from the auto-stuff.

In the meantime, perhaps you can use a script loader to load the Persona and django-browserid JS when the user clicks the button, and then trigger logout.

[skorokithakis]

I see, thank you for the explanation. Is there a reason django-browserid can't work that way by default? Inject include.js when a user clicks a button, and tell people to load it on the pages they want to support autologin. Sounds like a win-win scenario (although it would make users wait for include.js to load). Still, it sounds better than the current way.

[wrr]

Alternatively, we can just ignore any auto-login and auto-logout stuff by refusing to respond to anything that wasn't initiated by django_browserid.login and django_browserid.logout. This would break our ability to auto-login users who have just signed up for Persona for the first time, but it would help alleviate the pain we've gotten from the auto-stuff.

If you decide to do it, you can as well switch to navigator.id.get() instead of navigator.id.watch(). It also disables all auto stuff but additionally supports users with 3rd party cookies disabled.

[skorokithakis]

Hmm, but then my users won't be automatically logged in when they first sign up, will they? Still, it sounds enticing, thanks for the information.

[callahad]

@wrr .watch will soon support users with third party cookies disabled: mozilla/browserid#2999

@skorokithakis I've asked someone else from the Identity team to hop in here and discuss the rationale behind our current design, but I'll try to touch on two of your main points in the meantime:

it needs to load on every single page load, slowing down my page

Would it be enough to put the script at the end of your page body or mark it deferred so that it doesn't block or slow down the rest of your site?

It's also causing problems by apparently thinking that my session is expired and forces a logout, only to be led to the front page and automatically log in again. As you can imagine, this is a dealbreaker for users.

That sounds like something isn't configured quite right between your session management and the browserid API. Is there any way you could point me at your code so I can take a look and help out?

[skorokithakis]

Would it be enough to put the script at the end of your page body or mark it deferred so that it doesn't block or slow down the rest of your site?

I'm not sure, but I think Google counts the time until the full page load, so this would still count as "slow", plus, it's annoying to see the loading bar and makes the page feel slow.

Is there any way you could point me at your code so I can take a look and help out?

Sure thing, it's http://staging.getinstabot.com. I will set up an account for you for your gmail email now, you should be able to just log in. If you browse around/refresh a few times, it'll happen. The site's sessions are fine, Persona just thinks the user has opted to log out and logs me out.

I have third-party cookies disabled, if it helps diagnose.

[callahad]

I have third-party cookies disabled, if it helps diagnose.

Can you reproduce the issue with third party cookies enabled, or with an exception added for [*.]persona.org? This might be another instance of mozilla/browserid#2999, which should be fixed Very Soon by mozilla/browserid#3116.

[skorokithakis]

Oh, I already had the exception added quite a while ago, turns out. This was happening with the exception in place, so it's not that issue.

[skorokithakis]

I've changed the logout onclick code to:

e.preventDefault();

logoutRedirect = $(this).attr('href');

var head = document.getElementsByTagName('head')[0];
var script = document.createElement('script');
script.type= 'text/javascript';
script.src= 'https://login.persona.org/include.js';
head.appendChild(script);
script.onload = function () {
    navigator.id.logout();
}

I've verified that logoutRedirect is available when the script loads, and the logout() function gets called, but I am not logged out... What could be wrong? :(

[callahad]

Persona has to do some setup before things like id.logout and id.request can work, so you're racing that setup and probably triggering id.logout before it can handle it.

If you add an onready callback to id.watch, that will fire once Persona is ready and able to handle user-initiated actions. Not quite sure how to do that from within the framework of django/browserid, though. @Osmose ?

[skorokithakis]

Hmm, I'm not sure how to even do that, unfortunately... This could be a good candidate for inclusion in the default browserid.js file, though. My current code is:

// Call navigator.id.logout whenever a logout link is clicked.
$(document).on('click', '.browserid-logout', function(e) {
    e.preventDefault();

    logoutRedirect = $(this).attr('href');

    if (!navigator.id) {
        var head = document.getElementsByTagName('head')[0];
        var script = document.createElement('script');
        script.type= 'text/javascript';
        script.src= 'https://login.persona.org/include.js';
        head.appendChild(script);
        script.onload = function() {
            navigator.id.logout()
        }
    } else {
        navigator.id.logout()
    }
});

[skorokithakis]

Alright. Brace yourselves, because I have done it, and it works, and I don't know whether it's brilliant or horrible (and when you don't know whether something you did is brilliant or horrible, it's not brilliant).

I have tested the monster (if, admittedly, not thoroughly), on my site, and it behaves as expected. It loads the shim if it's not loaded, uses it if it is, logs out, logs in, and automatically logs in if the shim is loaded and the Persona session has not ended. As far as I can tell, it does the right thing on all circumstances, and removes the annoying logout bug (because it can't log out at its whim).

Behold:

/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

(function($) {
    'use strict';

    $(function() {
        // State? Ewwwwww.
        var loginRedirect = null; // Path to redirect to post-login.
        var logoutRedirect = null; // Path to redirect to post-logout.

        var $loginForm = $('#browserid-form'); // Form used to submit login.
        var $browseridInfo = $('#browserid-info'); // Useful info from backend.

        var requestArgs = $browseridInfo.data('requestArgs') || {};
        var loginFailed = location.search.indexOf('bid_login_failed=1') !== -1;

        var watchParams = {
            loggedInUser: $browseridInfo.data('userEmail') || null,
            onlogin: function(assertion) {
                // Avoid auto-login on failure.
                if (loginFailed) {
                    navigator.id.logout();
                    loginFailed = false;
                    return;
                }

                if (assertion) {
                    $loginForm.find('input[name="next"]').val(loginRedirect);
                    $loginForm.find('input[name="assertion"]').val(assertion);
                    $loginForm.submit();
                }
            },
            onlogout: function() {
                // Follow the logout link's href once logout is complete.
                var currentLogoutUrl = logoutRedirect;
                if (currentLogoutUrl !== null) {
                    logoutRedirect = null;
                    window.location = currentLogoutUrl;
                } else {
                    // Sometimes you can get caught in a loop where BrowserID
                    // keeps trying to log you out as soon as watch is called,
                    // and fails since the logout URL hasn't been set yet.
                    // Here we just find the first logout button and use that
                    // URL; if this breaks your site, you'll just need custom
                    // JavaScript instead, sorry. :(
                    currentLogoutUrl = $('.browserid-logout').attr('href');
                    if (currentLogoutUrl) {
                        window.location = currentLogoutUrl;
                    }
                }
            }
        }

        // Call navigator.id.request whenever a login link is clicked.
        $(document).on('click', '.browserid-login', function(e) {
            e.preventDefault();

            loginRedirect = $(this).data('next');
            navigator.id.request(requestArgs);
        });

        // Call navigator.id.logout whenever a logout link is clicked.
        $(document).on('click', '.browserid-logout', function(e) {
            e.preventDefault();

            logoutRedirect = $(this).attr('href');

            if (!navigator.id) {
                var head = document.getElementsByTagName('head')[0];
                var script = document.createElement('script');
                script.type= 'text/javascript';
                script.src= 'https://login.persona.org/include.js';
                head.appendChild(script);
                script.onload = function() {
                    watchParams["onready"] = function() {navigator.id.logout()}
                    navigator.id.watch(watchParams)
                }
            } else {
                navigator.id.logout()
            }
        });

        if (navigator.id) navigator.id.watch(watchParams);
    });
})(jQuery);

[Osmose]

w00t, awesome!

(FYI I won't be able to look at this for a week or so, currently tied up at a work week)

[ethernomad]

I'm the maintainer of the Drupal module: http://drupal.org/project/persona

I find no front end issue with having the Persona JS shiv loading on every page. When JS aggregation is enabled, Drupal automatically downloads include.js and serves it directly from site aggregated with all the other JS that needs to be on every page. This avoids setting up a HTTPS connection to login.persona.org before the "load" event.

I found that putting include.js at the bottom of the body rather than in head really slowed down page loads. There was a visible delay before Drupal's behaviours system kicked in.

I also found a solution to the bug in Persona where it will issue a rouge onlogout if you navigate away from the page too soon:

// Switch off Persona when leaving the page to prevent it from issuing a
// rouge onlogout if it hadn't finished initialising.
// @see https://github.com/mozilla/browserid/issues/2560
$(window).bind('beforeunload', function (event) {
  navigator.id.watch({
    loggedInUser: settings.persona.email,
    onlogin: function (assertion) {},
    onlogout: function () {}
  });
});

Here is our JavaScript: http://drupalcode.org/project/persona.git/blob/HEAD:/persona.js

[skorokithakis]

That's very interesting, I'll try it, it might solve both my problems at once. Thank you very much for this.

[ethernomad]

See also https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.identity/1qlB1KdRISs

[skorokithakis]

Is there any progress on this? I'm currently having problems with the JS again and it'd probably be nice if there were more eyes looking at this, by maybe integrating it to the mainline?

[Osmose]

@callahad Given that Persona is going to be moving away from managing sessions with it's API, what do you think of moving django-browserid to the navigator.id.get API? Without the need to call watch on every page, we can allow users to only include the JS on pages that need it.

As for solving this particular issue, we'd then use a script loader of some kind to load include.js from our own JavaScript.

[callahad]

Don't move to .get -- it's not possible to make login work on Chrome/iOS, Windows Phone, or after email confirmation from the .get API.

[Osmose]

Looking back on this, I don't think adding built-in support for not loading the JavaScript on each page is a good idea.

The new session-less API still requires pages to register callbacks via navigator.id.watch because it's the only way for browser-based Persona support to know what code to call after a user tries to log in a site with Persona, as the browser can't just 'figure out' what JS the site wants to run without being told.

This does allow us to say "well, only include the JS on pages when the user is logged out" because, with the session-less API, there is no JavaScript to call for logging out. This is a valid point, but by that point it's likely the user has already cached the JS and won't be making more requests anyway. And it's trivial for sites using django-browserid to check for this and not include the JS themselves rather than build it into the library.

I think adding an example of how to avoid including the JavaScript on logged-in pages to the documentation is the better fix. We still make it easy to figure out, but don't have to bother making our code more complex. Part of #201 involves adding a User Guide with examples of common usage patterns for django-browserid, and this would be a great fit.

Thoughts?

[skorokithakis]

The problem I can see is that you have to load the JS if you want to log out. For sites that have a "log out" link on every page, you have to eventually load the JS on every page or you won't be able to log out...

[Osmose]

Here's a link to the PR with the new stateless API: https://github.com/mozilla/persona/pull/3961

When using that new API (which hasn't landed in production yet but is oh-so-close), you don't have to call navigator.id.logout. This means you only have to log users out on your own site.

Because we switched to using ajax-based logins and logouts, without any changes you'll still need to load browserid.js from our library, or write some custom code to make the proper logout request. However, you can also make a view that calls django.contrib.auth.logout yourself, and redirect users to it with a normal requests, and then redirect them away. The built in logout view will do this without any issues and should work fine.

My suggestion is to include an example in the docs where you:

1. Add a conditional to the template code where you include the JS that doesn't include it if the user is logged in.
2. Add an entry to your URLConf that points to the built-in logout view.
3. Add a logout link that points to that view.

Those three steps should be all you need to have JS-less logouts. Given that they're not terribly hard and don't benefit much from being built-in to the library, I think documenting them instead of making it built-in somehow is the way to go.

I hope that makes my previous comment a bit more clear. :D

[skorokithakis]

Ah, thanks for that, I wasn't aware of that API. This is how I was doing it initially, then you guys implemented stateful logins and I had to call logout() in JS, now you're switching back to the first way again? :P

Since I'm not familiar with the API, I assume it's similar to the first one, where you just need to include browserid.js in pages with "login" buttons, which sounds great to me. Thanks for your help!

[Osmose]

Ah, thanks for that, I wasn't aware of that API. This is how I was doing it initially, then you guys implemented stateful logins and I had to call logout() in JS, now you're switching back to the first way again? :P

That's what we in the business like to call "listening to user feedback". :P

Since I'm not familiar with the API, I assume it's similar to the first one, where you just need to include browserid.js in pages with "login" buttons, which sounds great to me. Thanks for your help!

Yeah. If I'm reading that PR correctly, you just don't pass an onlogout handler to navigator.id.watch and don't call navigator.id.logout and then you don't have to worrry.

[skorokithakis]

Sounds great, thanks!

[Osmose]

Well, it took half a year, but the new stateless API has landed and there's a PR that has us switch over to it: #255

I believe that, as of that PR, you can safely only include api.js on a page with logout-only buttons, and call django_browserid.logout to trigger logout. browserid.js still requires it because it calls navigator.id.watch, so to avoid having to load include.js you'd have to write a custom JS file instead of relying on browserid.js, but that's not terribly difficult and I'm fine with just documenting it instead of trying to change browserid.js to support that.

[Osmose]

I can confirm that api.js works for logout without include.js. I'm not sure if we should bother documenting it though; it seems like a bit of an edge case to me.

@willkg @peterbe Thoughts?

[willkg]

I don't have any opinion one way or the other.

[Osmose]

Closing as an edge case. If it comes up as an issue again we'll reconsider.

[skorokithakis]

I would like to resurrect this. Persona makes seventeen requests on page load, even though the vast majority of my users will never log in. This is unacceptable to me, and I would like to dynamically inject include.js (or whatever is making the requests) after the user has clicked the login button. Is there a way to do this?

I don't necessarily need this to be in django-browserid, I just need some JS code that will do it. I tried hacking it on my own with the updated version of the library but didn't manage to get it to work.

[Osmose]

I would like to dynamically inject include.js (or whatever is making the requests) after the user has clicked the login button. Is there a way to do this?

I tried making a sample for this and failed as well, because every method of loading the JS in response to a user click I can think of or find functions asynchronously. This breaks because the popup box won't launch outside of a user-initiated event handler, and the async script loading breaks out of that event.

Fixing this requires changes to Persona itself that I doubt will ever happen at this point. :(

Seventeen requests on page load seems excessive, though. What is Persona doing with seventeen requests?

[skorokithakis]

It seems to be loading the images for the pop-up. That's odd.

[Osmose]

wat

why

why would it do that

[skorokithakis]

Maybe I implemented it wrong? See here:

https://www.pastery.net/

I'm just using django-browserid's form, though.

[Osmose]

I only see about 5 requests from Persona on page load. Or is this for when you're already logged in?

[skorokithakis]

No, this is on a brand-new incognito window:

https://i.imgur.com/ASruDlR.png

It's loading the iframe, so the iframe loads all this. The problem is that the site will have maybe one out of ten thousand page views logging in, so it's very rare that these need to be loaded :/

[Osmose]

I still can't replicate your results. I don't get those resources being loaded. I can't think of a reason why you're seeing them and I'm not.

[skorokithakis]

Hmm, neither can this:

http://www.webpagetest.org/result/151217_2S_13ZJ/1/details

Must be something in my setup, sorry about that.