From 0df2034dc80138bb59b551db48622fd18c851dfb Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 26 Feb 2013 17:25:56 -0500 Subject: [PATCH] Fixes #264 - replaceState was wonky --- .../javascripts/discourse/components/url.js | 28 +- .../discourse/routes/discourse_location.js | 345 +++++++++--------- .../javascripts/discourse/views/topic_view.js | 10 +- 3 files changed, 199 insertions(+), 184 deletions(-) diff --git a/app/assets/javascripts/discourse/components/url.js b/app/assets/javascripts/discourse/components/url.js index 6526bcf64c3..87d67327d52 100644 --- a/app/assets/javascripts/discourse/components/url.js +++ b/app/assets/javascripts/discourse/components/url.js @@ -13,6 +13,18 @@ Discourse.URL = { // Used for matching a /more URL MORE_REGEXP: /\/more$/, + /** + @private + + Get a handle on the application's router. Note that currently it uses `__container__` which is not + advised but there is no other way to access the router. + + @method router + **/ + router: function(path) { + return Discourse.__container__.lookup('router:main'); + }, + /** Browser aware replaceState. Will only be invoked if the browser supports it. @@ -20,12 +32,19 @@ Discourse.URL = { @param {String} path The path we are replacing our history state with. **/ replaceState: function(path) { + if (window.history && window.history.pushState && window.history.replaceState && !navigator.userAgent.match(/((iPod|iPhone|iPad).+\bOS\s+[1-4]|WebApps\/.+CFNetwork)/) && (window.location.pathname !== path)) { - return history.replaceState({ path: path }, null, path); + + // Always use replaceState in the next runloop to prevent weird routes changing + // while URLs are loading. For example, while a topic loads it sets `currentPost` + // which triggers a replaceState even though the topic hasn't fully loaded yet! + Em.run.next(function() { + Discourse.URL.router().get('location').replaceURL(path); + }); } }, @@ -36,9 +55,6 @@ Discourse.URL = { It contains the logic necessary to route within a topic using replaceState to keep the history intact. - Note that currently it uses `__container__` which is not advised - but there is no other way to access the router. - @method routeTo @param {String} path The path we are routing to. **/ @@ -71,13 +87,13 @@ Discourse.URL = { } // If we transition from a /more path, scroll to the top - if (this.MORE_REGEXP.exec(oldPath) && (!this.MORE_REGEXP.exec(path))) { + if (this.MORE_REGEXP.exec(oldPath) && (oldPath.indexOf(path) === 0)) { window.scrollTo(0, 0); } // Be wary of looking up the router. In this case, we have links in our // HTML, say form compiled markdown posts, that need to be routed. - var router = Discourse.__container__.lookup('router:main'); + var router = this.router(); router.router.updateURL(path); return router.handleURL(path); } diff --git a/app/assets/javascripts/discourse/routes/discourse_location.js b/app/assets/javascripts/discourse/routes/discourse_location.js index 5c4cf69b1b1..ee1d4234425 100644 --- a/app/assets/javascripts/discourse/routes/discourse_location.js +++ b/app/assets/javascripts/discourse/routes/discourse_location.js @@ -1,194 +1,191 @@ /*global historyState:true */ -(function() { - /** - @module ember - @submodule ember-routing - */ - var get = Ember.get, set = Ember.set; - var popstateReady = false; +/** +@module Discourse +*/ + +var get = Ember.get, set = Ember.set; +var popstateReady = false; + +/** + `Ember.DiscourseLocation` implements the location API using the browser's + `history.pushState` API. + + @class DiscourseLocation + @namespace Discourse + @extends Ember.Object +*/ +Ember.DiscourseLocation = Ember.Object.extend({ + init: function() { + set(this, 'location', get(this, 'location') || window.location); + if ( jQuery.inArray('state', jQuery.event.props) < 0 ) + jQuery.event.props.push('state') + this.initState(); + }, /** - `Ember.DiscourseLocation` implements the location API using the browser's - `history.pushState` API. + @private - @class DiscourseLocation - @namespace Ember - @extends Ember.Object + Used to set state on first call to setURL + + @method initState */ - Ember.DiscourseLocation = Ember.Object.extend({ - init: function() { - set(this, 'location', get(this, 'location') || window.location); - if ( jQuery.inArray('state', jQuery.event.props) < 0 ) - jQuery.event.props.push('state') - this.initState(); - }, + initState: function() { + this.replaceState(this.formatURL(this.getURL())); + set(this, 'history', window.history); + }, - /** - @private + /** + Will be pre-pended to path upon state change - Used to set state on first call to setURL + @property rootURL + @default '/' + */ + rootURL: '/', - @method initState - */ - initState: function() { - this.replaceState(this.formatURL(this.getURL())); - set(this, 'history', window.history); - }, + /** + @private - /** - Will be pre-pended to path upon state change + Returns the current `location.pathname` without rootURL - @property rootURL - @default '/' - */ - rootURL: '/', + @method getURL + */ + getURL: function() { + var rootURL = get(this, 'rootURL'), + url = get(this, 'location').pathname; - /** - @private + rootURL = rootURL.replace(/\/$/, ''); + url = url.replace(rootURL, ''); - Returns the current `location.pathname` without rootURL + return url; + }, - @method getURL - */ - getURL: function() { - var rootURL = get(this, 'rootURL'), - url = get(this, 'location').pathname; + /** + @private - rootURL = rootURL.replace(/\/$/, ''); - url = url.replace(rootURL, ''); + Uses `history.pushState` to update the url without a page reload. - return url; - }, + @method setURL + @param path {String} + */ + setURL: function(path) { + path = this.formatURL(path); - /** - @private - - Uses `history.pushState` to update the url without a page reload. - - @method setURL - @param path {String} - */ - setURL: function(path) { - path = this.formatURL(path); - - if (this.getState() && this.getState().path !== path) { - popstateReady = true; - this.pushState(path); - } - }, - - /** - @private - - Uses `history.replaceState` to update the url without a page reload - or history modification. - - @method replaceURL - @param path {String} - */ - replaceURL: function(path) { - path = this.formatURL(path); - - if (this.getState() && this.getState().path !== path) { - popstateReady = true; - this.replaceState(path); - } - }, - - /** - @private - - Get the current `history.state` - - @method getState - */ - getState: function() { - historyState = get(this, 'history').state; - if (historyState) return historyState; - - return {path: window.location.pathname}; - }, - - /** - @private - - Pushes a new state - - @method pushState - @param path {String} - */ - pushState: function(path) { - if (!window.history.pushState) return; - this.set('currentState', { path: path } ); - window.history.pushState({ path: path }, null, path); - }, - - /** - @private - - Replaces the current state - - @method replaceState - @param path {String} - */ - replaceState: function(path) { - if (!window.history.replaceState) return; - this.set('currentState', { path: path } ); - window.history.replaceState({ path: path }, null, path); - }, - - /** - @private - - Register a callback to be invoked whenever the browser - history changes, including using forward and back buttons. - - @method onUpdateURL - @param callback {Function} - */ - onUpdateURL: function(callback) { - var guid = Ember.guidFor(this), - self = this; - - $(window).bind('popstate.ember-location-'+guid, function(e) { - if (e.state) { - var currentState = self.get('currentState'); - if (currentState) { - callback(e.state.path); - } else { - this.set('currentState', e.state); - } - } - - }); - }, - - /** - @private - - Used when using `{{action}}` helper. The url is always appended to the rootURL. - - @method formatURL - @param url {String} - */ - formatURL: function(url) { - var rootURL = get(this, 'rootURL'); - - if (url !== '') { - rootURL = rootURL.replace(/\/$/, ''); - } - - return rootURL + url; - }, - - willDestroy: function() { - var guid = Ember.guidFor(this); - - Ember.$(window).unbind('popstate.ember-location-'+guid); + if (this.getState() && this.getState().path !== path) { + popstateReady = true; + this.pushState(path); } - }); + }, - Ember.Location.registerImplementation('discourse_location', Ember.DiscourseLocation); + /** + @private -})(this); + Uses `history.replaceState` to update the url without a page reload + or history modification. + + @method replaceURL + @param path {String} + */ + replaceURL: function(path) { + path = this.formatURL(path); + + if (this.getState() && this.getState().path !== path) { + popstateReady = true; + this.replaceState(path); + } + }, + + /** + @private + + Get the current `history.state` + + @method getState + */ + getState: function() { + historyState = get(this, 'history').state; + if (historyState) return historyState; + + return {path: window.location.pathname}; + }, + + /** + @private + + Pushes a new state + + @method pushState + @param path {String} + */ + pushState: function(path) { + if (!window.history.pushState) return; + this.set('currentState', { path: path } ); + window.history.pushState({ path: path }, null, path); + }, + + /** + @private + + Replaces the current state + + @method replaceState + @param path {String} + */ + replaceState: function(path) { + if (!window.history.replaceState) return; + this.set('currentState', { path: path } ); + window.history.replaceState({ path: path }, null, path); + }, + + /** + @private + + Register a callback to be invoked whenever the browser + history changes, including using forward and back buttons. + + @method onUpdateURL + @param callback {Function} + */ + onUpdateURL: function(callback) { + var guid = Ember.guidFor(this), + self = this; + + $(window).bind('popstate.ember-location-'+guid, function(e) { + if (e.state) { + var currentState = self.get('currentState'); + if (currentState) { + callback(e.state.path); + } else { + this.set('currentState', e.state); + } + } + + }); + }, + + /** + @private + + Used when using `{{action}}` helper. The url is always appended to the rootURL. + + @method formatURL + @param url {String} + */ + formatURL: function(url) { + var rootURL = get(this, 'rootURL'); + + if (url !== '') { + rootURL = rootURL.replace(/\/$/, ''); + } + + return rootURL + url; + }, + + willDestroy: function() { + var guid = Ember.guidFor(this); + + Ember.$(window).unbind('popstate.ember-location-'+guid); + } +}); + +Ember.Location.registerImplementation('discourse_location', Ember.DiscourseLocation); diff --git a/app/assets/javascripts/discourse/views/topic_view.js b/app/assets/javascripts/discourse/views/topic_view.js index 8872b292bbf..41072934edd 100644 --- a/app/assets/javascripts/discourse/views/topic_view.js +++ b/app/assets/javascripts/discourse/views/topic_view.js @@ -69,14 +69,16 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, { }).observes('topic.highest_post_number'), currentPostChanged: (function() { - var current, postUrl, topic; - current = this.get('controller.currentPost'); - topic = this.get('topic'); + var current = this.get('controller.currentPost'); + + var topic = this.get('topic'); if (!(current && topic)) return; + if (current > (this.get('maxPost') || 0)) { this.set('maxPost', current); } - postUrl = topic.get('url'); + + var postUrl = topic.get('url'); if (current > 1) { postUrl += "/" + current; } else {