backbone icon indicating copy to clipboard operation
backbone copied to clipboard

Backbone.history.navigate( '/' ) strips slash

Open ellatrix opened this issue 10 years ago • 18 comments

If my root is set to /path/to/root/, Backbone.history.navigate( '/' ) should not navigate to /path/to/root.

See #2656 and especially 151bd73d26ab92de3c224ded656e535a06d7e85b.

ellatrix avatar Nov 20 '14 20:11 ellatrix

I sort of found a solution. It works but it's not very clean, right now it is a modified version of Backbone 1.1.2's History.navigate method so if the library is updated the extension might need to be different.

But yeah you just have to comment out if (fragment === '' && url !== '/') url = url.slice(0, -1); and it seems to work just fine.

Hope this helps!

_.extend(Backbone.history, {
    navigate: function(fragment, options) {
        // Cached regex for stripping urls of hash.
        var pathStripper = /#.*$/;

        // Changed History.started -> Backbone.History.started
        if (!Backbone.History.started) return false;
        if (!options || options === true) options = {trigger: !!options};

        var url = this.root + (fragment = this.getFragment(fragment || ''));

        // Strip the hash for matching.
        fragment = fragment.replace(pathStripper, '');

        if (this.fragment === fragment) return;
        this.fragment = fragment;


        // Don't include a trailing slash on the root.
        // Commenting out this line removes the trailling slashes
        // if (fragment === '' && url !== '/') url = url.slice(0, -1);

        // If pushState is available, we use it to set the fragment as a real URL.
        if (this._hasPushState) {
            this.history[options.replace ? 'replaceState' : 'pushState']({}, document.title, url);

          // If hash changes haven't been explicitly disabled, update the hash
          // fragment to store history.
        } else if (this._wantsHashChange) {
            this._updateHash(this.location, fragment, options.replace);
            if (this.iframe && (fragment !== this.getFragment(this.getHash(this.iframe)))) {
                // Opening and closing the iframe tricks IE7 and earlier to push a
                // history entry on hash-tag change.  When replace is true, we don't
                // want this.
                if(!options.replace) this.iframe.document.open().close();
                this._updateHash(this.iframe.location, fragment, options.replace);
            }

            // If you've told us that you explicitly don't want fallback hashchange-
            // based history, then `navigate` becomes a page refresh.
        } else {
            return this.location.assign(url);
        }
        if (options.trigger) return this.loadUrl(fragment);
    }
});

jaidetree avatar Dec 11 '14 03:12 jaidetree

Or extend the Router with:

navigate: function( fragment ) {;
    var options = Backbone.history.options, ret;

    if ( fragment === '' || fragment === '/' ) {
        Backbone.history.stop();
        Backbone.history.start( { pushState: options.pushState, root: '/' } );

        arguments[0] = options.root;

        ret = Backbone.Router.prototype.navigate.apply( this, arguments );

        Backbone.history.stop();
        Backbone.history.start( { pushState: options.pushState, root: options.root } );

        return ret;
    }

    return Backbone.Router.prototype.navigate.apply( this, arguments );
}

Much simpler. :)

ellatrix avatar Dec 11 '14 17:12 ellatrix

You can probably even get away with setting the router's root property directly.

akre54 avatar Dec 11 '14 18:12 akre54

@akre54 I wouldn't recommend that. History depends on root having a leading and trailing slash. Changing that assumption could cause some strange results.

@avryl I'm curious, is there a technical reason you want the slash included or is it purely aesthetic? It's fine if it's just aesthetic, but if there is a technical reason I'd love to hear it! :smiley:

braddunbar avatar Dec 11 '14 18:12 braddunbar

@braddunbar Purely aesthetic. All our URLs have a trailing slash, it'd be weird if the root doesn't. Removing the trailing slash from all URLs is not an option.

ellatrix avatar Dec 11 '14 19:12 ellatrix

If Backbone allows you to set a root with a trailing slash, it shouldn't behave differently.

ellatrix avatar Dec 11 '14 19:12 ellatrix

Got it! If it's just aesthetic, I'd prefer to drop the special case as well. shrugs

braddunbar avatar Dec 11 '14 20:12 braddunbar

I'd prefer to drop the special case as well.

I'm not sure what you mean. Is this a special case?

On Thu Dec 11 2014 at 10:56:24 PM Brad Dunbar [email protected] wrote:

Got it! If it's just aesthetic, I'd prefer to drop the special case as well. shrugs

— Reply to this email directly or view it on GitHub https://github.com/jashkenas/backbone/issues/3391#issuecomment-66687165.

ellatrix avatar Dec 13 '14 13:12 ellatrix

I'm listening for click events to see if the URL is a child of the root. Requiring a trailing slash on Backbone.history.root breaks this, because the default route ("") is not in the root.

Backbone.history.start({pushState: true, root: '/foo/'});
$(document.body).on('click', 'a', function (event) {
  if (event.currentTarget.pathname.indexOf(Backbone.history.root) === 0) {
    // This fails for the default route, because the URL in the browser will be '/foo', not '/foo/'
    // Using '/foo' to test could potentially lead to matches against URLs like '/food'
  }
});

bartram avatar Dec 15 '14 23:12 bartram

I was referring to this code.

braddunbar avatar Dec 16 '14 12:12 braddunbar

Is there a PR to address this?

jashkenas avatar May 13 '15 21:05 jashkenas

@jashkenas: https://github.com/jashkenas/backbone/pull/3443 would solve it, but is a breaking change.

jridgewell avatar May 13 '15 21:05 jridgewell

Hi, was wondering if I could help here. Is this still an open issue ?

rdquintas avatar Dec 15 '15 12:12 rdquintas

I'm trying to get a solution to this as well. I would like my routes to look like: domain.com/#/route1 as opposed to domain.com/#route1 and I cant figure out how.

qodesmith avatar Apr 08 '16 02:04 qodesmith

(Bringing issue back from the dead)

This does result in a technical wart if you're marrying Backbone to a server-side framework that requires trailing slashes. Right now with my Django code everything works, however Backbone modifies the URL to lose the trailing slash; the result is a request to Django which sends a redirect to include the trailing slash. So a completely unproductive round trip happens every time.

It does feel inconsistent to only strip a trailing slash when we match the "root" and not at any other time.

I have a code change that keeps or strips the trailing slash based on whether the root was set with one or not. Would a PR on that be welcome? It's 2 lines of code and then tests.

kdickerson avatar Mar 12 '20 21:03 kdickerson

Would a PR on that be welcome? It's 2 lines of code and then tests.

Happy to take a look, if you want to open the PR...

jashkenas avatar Mar 31 '20 16:03 jashkenas

Great, I have to push it through a corporate-review process before I can submit it, so that will take a little while; but it is happening.

Edit: I've just learned one of the required approvers is unavailable while under a shelter-at-home order; so this will be delayed until that order is lifted.

kdickerson avatar Apr 03 '20 21:04 kdickerson

Hey, @jashkenas, I'd really appreciate it if you could take a look at the PR for this. Thanks.

kdickerson avatar Sep 30 '20 16:09 kdickerson