asual / jquery-address

jQuery Address - Deep linking for the masses
http://www.asual.com/jquery/address
GNU General Public License v2.0
817 stars 175 forks source link

$.address.change() fire on load, causing page to load twice #95

Open BMCouto opened 13 years ago

BMCouto commented 13 years ago

Hi all,

First of all I feel like this is a common problem, still I haven't found a fix for it. When doing:

    $.address.init(function(event)
    {
        // initializes the plugin
        $('a:not([href="#"])').address();             
    }).change(function(event)
    {
        $.address.state('/templates');
        var value = $.address.state().replace(/^\/$/, '') + event.value;
                //...
        }

The page gets loaded twice and then have the normal behavior. I fixed it changing to internalChange() but I don't believe it's the right way to do it. Any thoughts on this?

Thanks in advance!

asual commented 13 years ago

The $.address.state() is not supposed to used like this. It only sets the base address for HTML5 pushState management and should be used only once. By initializing the plugin you no longer need to do anything but only load the appropriate content when a change event occurs.

BMCouto commented 13 years ago

@asual still, without it, the content gets loaded twice at the first time cause the change() is fired when I go to the page for the first time... how can I avoid this?

And btw, without state(), wich url should I pass and where? All of your examples use ?state=/whatever

asual commented 13 years ago

Do you monitor the same behavior with any of the provided samples?

BMCouto commented 13 years ago

No, but I also don't see a common example, with normal url's (eg: /folder/file.html) without $.address.state(); Can you provide one or tell why is change() fired when accessing /folder/site.html directly? I saw a few more people with the same problem (and without a fix) so I believe I'm not the only one.

BMCouto commented 13 years ago

I tried another approach without the $.address.state(), still I have the same issue, change is fired when I load the page for the first time, and if i go directly to site.com/folder1/file.html and click on a link i got something like site.com/folder1/file.html#/folder1/file2.html ...

Thing is, the change() method is fired on the first time you visit the page, and should have something to avoid that, cause yes I need it for browser's back and forward, but it doesn't make sense to happen when you're accessing the site for the first time.

@asual can you please explain what should I use for this "common" behavior?

Thanks

barrychapman commented 12 years ago

I also have this same problem. When I load my site initially (fresh load) it loads the page twice.

Normal navigation is fine however. Has anyone found a fix?

Thanks

joloco commented 12 years ago

I've had this issue too, and have come up with the following kludge... (loadPage is my AJAX page loading function)

var addressChanged = false;  // This var will be set to false until after the first address.change call

$(document).ready(function() {

    $.address.change(function(e) {
        if (addressChanged) loadPage(e.value);  // Don't AJAX-load a page the first time round!
        addressChanged = true;  // Mark the first address.change as happened now
    });

});

I hope this helps someone!

joloco commented 12 years ago

Of course, that prevents deep linking, so perhaps add this second line...


    var addressChanged = false;
    if (window.location.hash) addressChanged = true; // If there's a hash, do the AJAX page load
BMCouto commented 12 years ago

I' ve made this, still the behavior isn't very good on IE:

    var firstTimeOnSite = 1;

    $.address.state($basePath).init(function() {
        // Initializes the plugin
        $('a:not([href*="#"],[target="_blank"])').address();
}).change(function(event)
{
    // selects the proper navigation link
    $('a:not([href*="#"],[target="_blank"])').each(function() {
        if ($(this).attr('href') == event.pathNames[0]) {
            $(this).addClass('active');
        } else {
            $(this).removeClass('active');
        }
    });

    //if first time on page set the current address value
    if(firstTimeOnSite == 1 && event.path=='/' && location.pathname!=='/')
    {
        var value = location.pathname;
        value = value.replace('//', '/');
        firstTimeOnSite=0;
        //return this;
    }
    else
    {

        var value = $.address.state()+ event.path;
        value = value.replace('//', '/');
        firstTimeOnSite=0;
    }

    //loads the page content and inserts it into the content area
    $.ajax({
            url: value,
            beforeSend: function()
            {
                $pageContainer.find('#loadingContainer').css({'opacity':'0', 'display':'block', 'margin-top':'0'}).animate({'opacity':'0.8'});
            },
            error: function(XMLHttpRequest, textStatus, errorThrown) 
            {
                    //handler(XMLHttpRequest.responseText);
                    handler($dynamicStrings.errorTemplate);
            },
            success: function(data, textStatus, XMLHttpRequest)
            {
                handler(data);
            }
    });

});

@asual do you have any better solution for this?

paulm17 commented 12 years ago

There is also really serious issues when you do a redirect to another page and have the state as / and the new page is /something.

For example. Lets say you are in /login and you authenticate and redirect to /accounts. You redirect but the state is / and when the page loads and fires it seems to want to overwrite the url to / which then I have a redirect for logged to /accounts instead.

So guess what, I get an infinite loop for / goes to /accounts and the script fires /accounts to /.

I only get this issue with POS IE. I only get the firing twice with IE.

I have noticed that the samples are broken from a usability pov. For example, if you have /home, /marketing, /something and home is the first page. Well when you are in /something and hit refresh, you quickly see /home and then the script fires and goes to /something. That's pretty weak and pretty crappy as users will complain about this and as a dev, well I am complaining about it.

I have no issues with Chrome, Firefox, Opera and Safari. Just chrome.

@asual, can you please take a look at this? It's been broke for IE for about a year now. :@

Edit: Looking at github and gizmodo who use some html state, for non-IE browsers it's in effect. For IE they still use redirect. I think I might just do the same :'(

asual commented 12 years ago

I hope that I'll find some time soon to look into this issue and resolve it for 1.5.

@no1youknowz Can you please explain how you set /home to be the first page? In general I don't think that using something different from / is good for the root page. Do you experience this with all IE versions?

paulm17 commented 12 years ago

@asual: having something like /accounts is perfectly fine for the root.

I'm using codeigniter and it's own routing system to have something like http://www.domain.com/accounts.

I am seeing this issue with IE7+.

Please don't get my tone wrong. I am tired after working since 8am and now it's 11.30pm lol. I love jquery address and it's working perfectly for all other browsers. :)

asual commented 12 years ago

Can you please elaborate a little bit more. You want to have

http://www.domain.com/accounts and http://www.domain.com/accounts#/home or http://www.domain.com/#/accounts and http://www.domain.com/#/something

Do you use the pushState support or just the standard old school deep linking?

paulm17 commented 12 years ago

Here is an example navigation through my site:

www.domain.com <-- initial page www.domain.com/classifieds <-- classifieds section http://www.domain.com/classifieds/cars-vans-motorbikes <-- Auto section http://www.domain.com/classifieds/cars-vans-motorbikes/54-renault-master-25d/7 <--- to look at an advert

http://www.domain.com/login <-- login page http://www.domain.com/accounts <-- accounts page http://www.domain.com/accounts/editprofile <-- edit profile page

My site is setup that I do not need to rely on jquery address for deep linking as with javascript turned off, web crawlers will still be able to navigate through the site. I only really need the pushState support.

Here are some pics of the site under chrome. As you can see I have no # or !# in the uri: http://imgur.com/id255,i5LR7,5vObV

Under IE, I would have www.domain.com/!#/classifieds <-- classifieds section

My code below:

// pageLoaded is false initially.

$.address.crawlable(1).state('/').init(function()
{
    $('a.address:not([href^=http])').address();            
}).internalChange(function(e)
{
    var url = (e.path == '/' && $('.userpiccontainer').length ? '/accounts/' : e.path);

    do_ajax_process(url);

    return false;
}).externalChange(function(e)
{
    // MS IE is dumb, so we need to deliver a blank page and then pull in content even the first time
    if (bp.getBrowser() == 'msie')
    {
        var url = (e.path == '/' && $('.userpiccontainer').length ? '/accounts/' : e.path);

        do_ajax_process(url);

        return false;
    }
    // ExternalChange fires on page refresh (for Chrome, FF, Opera, Safari) and browser button
    // pageLoaded is set to true in template main.php so it doesn't fire on first render
    else if (pageLoaded == false)
    {
        var url = (e.path == '/' && $('.userpiccontainer').length ? '/accounts/' : e.path);

        do_ajax_process(url);

        return false;
    }

    pageLoaded = false;
});

Edit: I forgot to mention. When jquery address is initalised, then all navigation through the site is done via jquery address/ajax. I only rely on the URI when the user hits refresh and the webserver can then direct them to the proper page and not having to rely on jquery/ajax.

asual commented 12 years ago

First, please do not use the crawlable feature since the site already exposes the content for indexing in a better way. I'm seriously considering dropping this option completely because I don't think it makes sense anymore.

Can you reproduce the issue with http://www.asual.com/jquery/address/samples/state?

paulm17 commented 12 years ago

No, I can't reproduce this issue that I have with the sample.

With Chrome I have:

http://www.asual.com/jquery/address/samples/state/ <-- home http://www.asual.com/jquery/address/samples/state/about etc

With IE I have:

http://www.asual.com/jquery/address/samples/state/# http://www.asual.com/jquery/address/samples/state/#/about

However, your state is the same throughout:

$.address.state('/jquery/address/samples/state').init(function() {

But for my own purposes, the state will always be different.

asual commented 12 years ago

The state should always be the same. It's not a variable that changes but an initialization parameter that informs the script about the root folder of the page. In your case it's / and in the case with the sample it's /jquery/address/samples/state. It's needed so that the script can handle pushState and Ajax correctly.

paulm17 commented 12 years ago

Sure, that's fine. I still have my redirection issue though. Until this IE firing duplication issue is solved.

asual commented 12 years ago

I will check if the behavior for IE can be optimized but it will never be the same as in pushState enabled browsers. Meanwhile it will be great if you can download the latest State sample and edit it so that can demonstrate the problem.

paulm17 commented 12 years ago

Thank you, I do appreciate that.

Yes, I will grab the latest sample and see if I can duplicate my problem with that.

paulm17 commented 12 years ago

Looking at the sample, there really is no difference between the state example and my code.

I have figured out my problem and it was due to the redirect infinite loop issue. Let me explain.

When I am at http://www.domain.com/#/login and then the site redirects to http://www.domain.com/accounts, well the redirect occurs, however IE is determined to do another refresh and change the url to http://www.domain.com/#/accounts. Guess what, the server will never see /#/accounts and therefore thinks it's just /. In my backend code I have if at root and logged in, redirect to /accounts. So the cycle starts again, it goes to accounts, IE wants to refresh the url, only sees / and LOL.

So, IE is really at fault here for being a) crap browser and b) wanting to refresh the URL on an externalChange.

Granted, this works fine for internal changes fine. I have no problems here.

I have updated my site, to take into account users who use IE 7-9. It's a bit of a mess, but now I don't have issues. It's a shame but this should really be fixed in 1.6.

Thanks for the plugin tho. It really did wonders for my site :)

asual commented 12 years ago

In IE we can only change the address from /accounts to /#/accounts by refreshing the page. The #/accounts hash is only available in the browser and it's not sent to the server. This doesn't leave us with many options here.

You can try fixing the issue by detecting IE on the server and directly redirect to /#/accounts. Alternatively you can support this deep linking only for pushState enabled browsers.

paulm17 commented 12 years ago

The issue here is not about /accounts or /#/accounts or any redirecting. The issue is this:

In Chrome: When I navigate to http://www.domain.com/accounts, the page only loads once.

In IE: When I navigate to http://www.domain.com/accounts, IE changes the URI to http://www.domain.com/#/accounts, which then jquery address executes externalChange function.

I cannot explain the issue more clearly.

The problem is with IE. It's been explained by other posters and there are two issues on this now.

My advice is to please look at the issue. If you cannot resolve it, then please put in the FAQ that IE causes this issue.

Thanks

asual commented 12 years ago

As I said before the address in IE cannot be changed without an extra request. If you want to have a clean hash based deep linking fallback in IE then you have to accept this and eventually implement a some sort of a workaround on the server.

Possible other options are:

paulm17 commented 12 years ago

I really suggest you update your website with this information for IE. Otherwise the lack of information will only serve to infuriate devs working on IE and thinking this is a jquery address issue.

asual commented 12 years ago

I did a quck hack that should replace the first request with a 302 redirect. It should work in all IE versions before 10. Check the network traffic for http://www.asual.com/jquery/address/samples/state-ie

paulm17 commented 12 years ago

Looking at the source, they seem to be identical. Was it changes to .htaccess or jquery address. Can you please be more specific on what you did? Thanks

asual commented 12 years ago

The change is in the PHP file and I'm wondering if I should commit it. You can compare the behavior of the original sample and this one.

I did some refactoring but this is the important part:

if (preg_match('/MSIE\s(?!10)/i', $_SERVER['HTTP_USER_AGENT']) && 
    $_SERVER[ 'HTTP_X_REQUESTED_WITH' ] != 'XMLHttpRequest' && 
    Address::value() != '/') {
    header('Location: ' . Address::state() . '/#' . Address::value());
    exit();
}
paulm17 commented 12 years ago

No, doesn't work for me. I am using codeigniter and doing will be the same as your code:

redirect('/accounts/#home', 'refresh'); 

Regardless, I am happy with the other hack I have done and so far I have had no problems and now don't mind the extra double hit with IE as it only happens on a page refresh with F5.

Paxxil commented 12 years ago

My solution for non pushState browsers was to put

if  ($http_user_agent ~ "MSIE [0-9]\.|Firefox/[0-3]\.|Chrome/[0-9]\.|Version/[0-4]\.(.*?)Safari|Opera(.*?)Version/([0-9]|10)\.")
{
    rewrite ^/(.*)$ /#/$1 redirect;
}

in nginx configuration file. If you want crawlable version add #! instead of #

EDIT: only check for IE6 - IE9 instead of all IE versions EDIT2: added all other non pushState browsers

asual commented 12 years ago

@Paxxil Thanks for sharing this! I'm planning to release 1.5 with the PHP fix mentioned above. Do you think it will work for you? In general we don't want such checks to include IE10 because it supports pushState.

mscarda commented 11 years ago

Hello, I'm still having the twice visit page problem.

Any help pls?

vicmosin commented 11 years ago

Hello, I am still have change fire on load... Chrome 22.0.1229.94, Win7x64 Is there any solution? Thank you

asual commented 11 years ago

@nKognito The change event is designed to fire on load. If you need to skip the first firing then you can use a simple flag that you can switch one during the first change.

vicmosin commented 11 years ago

Understood. Thanks for your response.

rrolla commented 10 years ago

I have the same problem, but the reason was that i include jquery.address js file twice :)

jacobjoy24 commented 5 years ago

@rrolla Thanks for your suggestion. Mine too got bundled twice.