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

navigate() with a space in the url triggers route even when {trigger:false} in Firefox #3399

Closed
chaimpeck opened this issue Nov 27, 2014 · 8 comments
Labels

Comments

@chaimpeck
Copy link

Consider this example:

$(function(){
var AppRouter = Backbone.Router.extend({

    routes: {
        '(count/:count)': 'home',
    },

    home: function(currCount) {
        var counter = 0;
        var thisObj = this;

        $("body").html($("<p>Click me</p>").on('click', function() {
            console.log("clicked "+counter);
            counter++;
            thisObj.navigate('count/'+counter, {trigger:false, replace: true});
        }));

        console.log('home route: '+currCount);    
    },
});

window.app = new AppRouter();
Backbone.history.start();
});

This should log "home route: null" and then proceed to log "click #" for every click on "Click me".

Now, make one small modification. Add a space to the url in navigate():

thisObj.navigate('count/ '+counter, {trigger:false, replace: true});

Try again and you will find that that "home route: #" will also start being logged, and the counter will reset two out of every three clicks.

This is in the 1.1.2 release and I am testing on the latest Firefox 33.1. I am not having this issue in Chrome - it could be because Chrome maybe does something to encode spaces in the URL. I'm not sure though.

I realize that the space character should not be in a URL in the first place, but this behavior is odd and should at the very least be documented. (i.e. "The navigate() may not behave as expected with URLs containing spaces (or other unsafe characters??).")

I am not sure if there is an easy fix for this, but I hope that at least by documenting this bug I save other developers time in trying to hunt it down.

@braddunbar
Copy link
Collaborator

Hi @chaimpeck! Thanks for reporting this.

Unfortunately, I was not able to reproduce it using the code above. I tried the latest Chrome and Firefox with both the master branch and the 1.1.2 release. Would you mind posting a working example in a gist/bin/fiddle that I can take a look at?

We've definitely had problems with encoding/decoding specific characters in Firefox before and I wouldn't doubt there are more. 😃

@ScoobyTchotchke
Copy link

ScoobyTchotchke commented Sep 8, 2016

I actually have seen this exact same issue, but am unable to share my code. I'll see if I can mock up some test code to replicate this, but I've discovered that the issue is most definitely browser based. All modern versions of IE and Chrome work fine, but at least on Firefox v31.5.0 (don't ask, I hate the infrastructure we're stuck on), it definitely triggers without even adding the trigger option.

@ScoobyTchotchke
Copy link

ScoobyTchotchke commented Sep 8, 2016

Here's what I've discovered. In Firefox, if you check the variable "this.location.href", any whitespaces in the variable are encoded into %20's. Chrome (and presumably IE) leaves them as whitespace. This is an issue in Backbone, since when you go to navigate, "checkUrl()" does a comparison of "this.fragment" against the returned value of "this.getFragment()" to determine whether or not to call "this.loadUrl()", if they don't match. this.getFragment() returns the value of this.location.href, with the %20's in place of any spaces in Firefox (or any encodeings for that matter). this.fragment returns an decoded equivalent. this.fragment remains decoded, regardless of whether or not you use encodeURI() on your URL's, because in the "navigate()" function, this.fragment is assigned by calling "this.decodeFragment()" on the URL that gets passed in, removing any attempts at forcing encoding.

Therefor, if this.fragment is always a decoded URL due to the use of this.decodeFragment(), and the URL contains any characters that encodeURI() would change (in this case spaces), it will never match the returned value of this.getFragment() in Firefox, since Firefox encodes the value of this.location.href, whereas other browsers don't. Since these two values don't match, this.loadUrl() gets called in checkUrl(), regardless of whether or not the trigger parameter in router.navigate is true or false.

The preliminary fix (pre-exhaustive testing) would be to change the return value of the "getHash()" method to read:

return match ? this.decodeFragment(match[1].replace(pathStripper, '')) : '';

@Dahkon
Copy link

Dahkon commented Mar 14, 2018

Still needed with last version so thank you !

@Amsvartner
Copy link

Amsvartner commented Mar 19, 2018

I have the same issue when trying to use non-standard characters in the URL, in my case åäö (characters that are in the Swedish alphabet). Latest version of Chrome and Backbone.

sjmiller85's solution worked for me as well. Thanks a bunch!

@Aggror
Copy link

Aggror commented Apr 4, 2018

@sjmiller85 What was your solution here? I do not fully understand what should be used for 'pathStripper'. Is this a regex?
Edit: this works in my situration. see https://github.com/jashkenas/backbone/pull/3955/files
getHash: function (t) { var e = (t || this).location.href.match(/#(.*)$/); return e ? e[1] : "" }

@Amsvartner
Copy link

@Aggror pathStripper is, according to the documentation, a cached regex for stripping urls of hash.

// Cached regex for stripping urls of hash.
var pathStripper = /#.*$/;

So it's the equivalent of your solution, only more neat and efficient :)

@abhiishere765
Copy link

avoid adding the simultaneous same hash to your router and also take care of initial condition , I will show you a code
(to handle initial case) (to prevent adding simultaneous same routes)
if ( routes.length === 1 || routes[routes.length-1] === getHash() ) {
return false;
}
else{
routes.push(getHash())
};

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

8 participants