Ember.AutoLocation support

[jayphelps]

The original PR is here #2685.

• Initial implementation
• Consensus on halting Router setup
• FEATURE flagged
• Unit tests - 1. how do we reliably test this since we can't actually redirect the test host environment? Mock the window.location object to override .replace()? 2. The only current Location class testing is in /ember/tests/routing/basic_testing.js, but it's only testing HistoryLocation and not much coverage. I think a separate PR is needed to add coverage to all Location classes..Thoughts?

Some people would like to support the history API but auto fallback to hashchanges in older browsers. The current way they could do that is check for support themselves and dynamically set the location property when they reopen the Router, but this makes it easy for them to not need that boilerplate code and also will transform between the two URL forms for sharing compatibility.

With the AutoLocation class, users can now just use:

App.Router.reopen({
  location: 'auto'
});

Let's discuss.

[wagenet]

Just a small note, it won't be 1.1+ it will be whatever release it makes it into, which at this point won't be any earlier than 1.3 and probably not until 1.4. Also, can you follow the guidelines here: http://emberjs.com/guides/contributing/adding-new-features/

[jayphelps]

I didn't mean to imply it would make into 1.1, just that the code been updated to work with 1.1+ as of now. I'll change the title to remove the confusion. Thanks.

[jayphelps]

As I presume multiple changes will be requested, I'm intentionally holding off on FEATURE flagging, commit naming conventions and tests until the implementation is agreed to be solid so I don't have to repeatedly update things. Biggest concern is the ability to halt the Router/app from continuing, as noted.

[wagenet]

@jayphelps Fair enough.

[trek]

I'm still +1 on this generally. Glad to see you're back at it!

[salzhrani]

+1

[jayphelps]

@trek Any suggestions on canceling the initial Router setup so AutoLocation can redirect the page without the wrong route rendering? I put in a hacky way as an example here

[trek]

Hey @jayphelps you still mucking around with this?

[jayphelps]

@trek was waiting on someone from core to chime in on how to halt the Router's setup so there's no render flash before location.replace, etc.

Been pretty much maintaining a diff at @pivotshare that we apply to new releases since we use this feature in prod.

[wagenet]

@machty ping.

[jayphelps]

To clarify, I assume the way I'm canceling the the Router start process here is too hacky so I'm waiting for input.

[machty]

@jayphelps I'm having trouble following why you need some kind of redirect or whatever it is that necessitates the canceling of the router setup. Can you please explain why this is necessary in more detail or head to #emberjs-dev in freenode to hash it out?

[machty]

@jayphelps explained the situation to me:

If someone with History enabled navigates to http://yoursite/about/faq and shares that URL with someone on IE9 with only Hash location, IE9 user will go to that URL and AutoLocation will detect that the user is trying to use a History location URL, at which point our implementation of AutoLocation can do one of two things:

1. Display the URL as http://yoursite/about/faq#/about/faq
2. Use location.replace to do a full page refresh to http://yoursite/#/about/faq

@jayphelps took the #2 approach, which was resulting in a momentary flash of the fully-rendered app before the browser reload kicked in. I can help work out a way to prevent this flash in a non hacky way, but the question still stands: which of 1 or 2 do we want to support? Do we make it configurable? #1 is definitely ugly, but I've seen both Facebook and JSBin employ this approach, so it might be totally fine? What do people think?

[davidpett]

I am running into the same issue, #1 is ugly, and using history.js (not what I want to use) to accommodate will still concatenate the hashed url after the deep link, this gets more and more messy as people share the url. I really like approach #2 to maintain pretty urls and also to be compatible in all browsers. The page "flash" should be quick since by this time the browser will cache the loaded asset files.

[jayphelps]

@machty 👍 exactly.

If anyone wants to know what option 2 looks like (location.replace refresh), we've been using it in the wild for many months. Here's an example: https://www.fitfusion.com/authors

Visit using IE9, and you'll see it get transformed to /#/about

[machty]

@jayphelps You've convinced me to support 2. I don't actually think your solution is all that hacky. I can't really think of a less hacky place to put it. I'll think on this some more. In the meantime, can you rebase against master and squash your commits?

[jayphelps]

@machty done.

[jayphelps]

@machty So I know, what more does this need to land in master?

[machty]

Boom. There may be a few more things to tweak after our weekly go/no-go meeting for feature flagged stuff but looks good to me.

[jayphelps]

Woohoo! Thank you!

[stefanpenner]

these docs should also illustrate that serverside support is needed in some situations..

[jayphelps]

@stefanpenner ohh yes. Definitely.

[davidpett]

excited to see this land and will be using right away. great work @jayphelps

[jayphelps]

Thanks @davidpett!

[tomdale]

💋 for @jayphelps

[jayphelps]

@tomdale don't get fresh with me, sonny.

[rlivsey]

Just noticed a problem when trying this out.

We have a route for oauth (implicit grant) logins which our users are redirected to when coming back from logging in as: /login/oauth#token=123456.

This is being transformed into /login/oauthtoken=123456 (same as above without the #) which then breaks because there's no oauthtoken route.

Everything works again when going back to the history location type.

I'll see if I get time later to dig into what's going on.

[wagenet]

It seems like it's attempting to transform your URL to a history state. Since the default hash url is something like "http://myapp.com/#/foo/bar" it's just stripping the hash sign to give your result. Unfortunately, that's not what you want. You may need to customize your location handler to treat this case separately. I'm not sure if this is something we can account for universally.

[jayphelps]

@rlivsey @wagenet is correct, though #4098 may actually resolve this for you. The description there isn't the same, but the code changes required to support his may let you do something like this, which AutoLocation will need to be aware of as well.

[jayphelps]

@wagenet There's been some talk on IRC about forcing #/ as the route prefix, requiring the forward slash. If this can be adopted as a requirement, I can easily adapt AutoLocation's transform not to handle hashes in the URL that don't follow this convention?

[wagenet]

@jayphelps That seems reasonable as long as there aren't any backwards compatibility concerns.

[gpoitch]

@jayphelps we needed to add a check for a forward slash in getHistoryPath(). Entering the app via Google search results added a '#!' in some browsers, causing a redirect to an unknown route.

[jayphelps]

@gdub22 Which browsers are you seeing this in? Got an example we can reproduce? I've never seen Google modify the forwarding url...that's bizarre of them.

[gpoitch]

@jayphelps
It was only happening in mobile safari, and possibly android, iirc.
Since we patched it and been running AutoLocation for a while, I don't have a live example.
Here is the spec: https://developers.google.com/webmasters/ajax-crawling/docs/specification?csw=1

It's a similar situation to @rlivsey's and seems worth addressing. #4098 looks promising.

[jayphelps]

@gdub22 Could you clarify what the attempted path format was that freaked it out?

e.g.

/about#!
/about#!/about
/#!/about
/#/about#!/about
etc

I'm aware of the hashbang spec, but AFAIK Google doesn't assume you're using it, it's up to you to use, which Ember doesn't support (again, AFAIK)

[gpoitch]

@jayphelps
From search results on mobile:
Actual: /, redirected to: /#!, AL transformed to: /!
Actual: /about, redirected to: /about#!, AL transformed to: /about!

The solution was to check for '/' as the first char of the hash path, meaning a history browser was actually attempting to open a hash route.

Regardless of the google stuff, any url with a hash anchor added would break when shared between 2 history supporting browsers e.g. http://example.com/about#top

[jayphelps]

Btw anyone wanting to use this PR right now with v1.3.1 (latest stable) can get the builds here: https://github.com/jayphelps/ember.js/releases/tag/v1.3.1%2Bauto-location

[abobwhite]

I have been using this Pre-Release and after testing for a few hours, things seem to be working swimmingly

[gpoitch]

@jayphelps modernizr updated that dirty sniff for false positive history support: Modernizr/Modernizr@663a458

[jayphelps]

@gdub22 :sadface: Android 4.0 too? Come on Google.........

Thanks for the heads up. If you want to PR it, feel free. I won't be able to get to it until Sunday.

Btw, how did you notice that? Do you follow their commits in general or did you run into this on Android 4.0?

[rwjblue]

@gdub22 - could you file an issue for that? I don't want to loose track of it before we release 1.5.0.

[gpoitch]

@jayphelps @rjackson I was just watching that particular issue. I'd like to take another crack at a feature detect before completely ruling it out and just opening an issue with their update.

[gpoitch]

I started a discussion and a few attempts at an actual feature detect: Modernizr/Modernizr#1270