Needed host check for pushstate.js #249
Conversation
…ML5 pushstate only works on the same hostname. Links to other domains will now work.
…ix broke ie. A blank host really means the current host, so we can check to see if it's blank and assign the current host, then proceed.
| @@ -34,14 +34,17 @@ steal('can/util', 'can/route', function(can) { | |||
| _setup: function() { | |||
| // intercept routable links | |||
| can.$('body').on('click', 'a', function(e) { | |||
| if(can.route.updateWith(this.pathname+this.search)) { | |||
| // Fix for ie showing blank host, but blank host means current host. | |||
| this.host == '' ? this.host = window.location.host : null; | |||
There was a problem hiding this comment.
This looks a little weird. Isn't this just doing
if(!this.host) {
this.host = window.location.host;
}Besides that I'd merge this in because it makes sense. I was thinking about a good way for testing this but I don't think there is (even checking the window.location in an iframe wouldn't really tell us much).
There was a problem hiding this comment.
That does look better. I updated the pull request to match. Thanks.
Have a great day!
Marshall Thompson
Creative Ideal
On Wednesday, January 23, 2013 at 1:38 PM, David Luecke wrote:
In route/pushstate/pushstate.js:
@@ -34,14 +34,17 @@ steal('can/util', 'can/route', function(can) { > _setup: function() { > // intercept routable links > can.$('body').on('click', 'a', function(e) { > - if(can.route.updateWith(this.pathname+this.search)) { > + // Fix for ie showing blank host, but blank host means current host. > + this.host == '' ? this.host = window.location.host : null;
This looks a little weird. Isn't this just doing
if(!this.host) { this.host = window.location.host; }Besides that I'd merge this in because it makes sense. I was thinking about a good way for testing this but I don't think there is (even checking the window.location in an iframe wouldn't really tell us anything).
—
Reply to this email directly or view it on GitHub (https://github.com/bitovi/canjs/pull/249/files#r2748417).
Needed host check for pushstate.js
|
Cool, thanks for the pull request. Hopefully we can make this plugin officially a part of 1.2. You definitely got it a step closer :) |
|
I know the documentation needs some work. What else needs to be done? It's working quite awesome for me in my projects, now, but I did have to prepend my routes in the controllers with a forward slash /. I played around with removing the forward slash, but it messed up the URL in the address bar whenever they were more than one level deep. I'll work on getting it to work without changing the controller routes. Anything else you can think of? Have a great day! Marshall Thompson On Wednesday, January 23, 2013 at 3:41 PM, David Luecke wrote:
|
|
@daffl Where can I find some documentation on this pushstate code? I'm trying to use it but not getting very far. Reverse engineering isn't in the budget on this one :) |
|
@marshallswain: If you use it without any major problems, find a fix for the problem and would like to contribute documentation that would be greatly appreciated! Just create a @talentedmrjones: Unfortunately there isn't any yet. The only way at the moment is referring to the demo.html for usage. |
pushstate.js was only checking the pathname. Since HTML5 pushstate only works on the same hostname, other links should be allowed to work as normal. Links to other domains will now work. Also updated the variable name at lines 46 and 47 to be the pathname instead of the href, since that's what we are really working with at that point in the code.