Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Trailing slash in older browsers #1320

Closed
pvencill opened this issue Apr 22, 2016 · 21 comments
Closed

Trailing slash in older browsers #1320

pvencill opened this issue Apr 22, 2016 · 21 comments

Comments

@pvencill
Copy link

When a browser (e.g. IE 9) does not support the History API, and therefore Angular falls back to using the #! in the URL. However, the home page URL needs to therefore be /#!/ for routing to work properly, and core.client.routes.js does this:

$urlRouterProvider.rule(function ($injector, $location) {
      var path = $location.path();
      var hasTrailingSlash = path.length > 1 && path[path.length - 1] === '/';

      if (hasTrailingSlash) {
        // if last character is a slash, return the same url without the slash
        var newPath = path.substr(0, path.length - 1);
        $location.replace().path(newPath);
      }

This means users can only navigate to the home page in those browsers if you add in the final / after the #!. the trailingslash logic needs to be updated to account for it if pre-HTML5 browses are supported.

@ilanbiala
Copy link
Member

@pvencill we aren't actively trying to support older browsers, ideally we only support evergreen browsers as that covers most of the user base.

@trendzetter
Copy link
Contributor

@pvencill Why should meanjs support browsers that are deprecated by the vendor?
https://www.microsoft.com/en-us/WindowsForBusiness/End-of-IE-support

@pvencill
Copy link
Author

So, I get that line of thinking, but I was looking at it from the other direction. Why should meanjs actively break something that AngularJS supports, without a good reason? Especially since it's such an easy fix. Either test for the support of pushstate as part of the logic I posted above, or use a regex to detect that the URL is the root URL of the site. Using something like:

var reg = /^(https?:\/\/)?([\da-z\.-]+)\.([a-z\.]{2,6})(\/|\/#!\/)$/;
var isSiteRoot = reg.test($location.absUrl()); 

Will tell you that.

Seems like a pretty small price to pay to avoid breaking an AngularJS feature. Sure, it's easier to support only the modern browsers, but some clients might not want to give up on 12% of the market share globally (and there are segments e.g. in world governments that have a higher % usage of older browsers).

@trendzetter
Copy link
Contributor

trendzetter commented Apr 28, 2016

I must admit that I am not fan of the trailing slash removing behavior. I don't need it and I had to remove it because I make use of the possibility to have a trailing slash in the application I build on meanjs. I never understood why meanjs wants to remove those slashes anyway. Because someone has build an application that cannot handle it? it appears. I think it should not be in the framework, overhead that is not needed, just in the application that has an issue with trailing slashes, and I wouldn't be surprised if that application is doing something questionable if it is ending up with the incorrect url. Not a framework issue if you ask me.

Source PR:
#1238

Source issue:
#1075

@mleanos you mind weighing in on this?

@trendzetter
Copy link
Contributor

I am starting to think that this hack to remove the trailing slash has been adopted too lightly. Just another issue with it:
#1224 (comment)

@ilanbiala
Copy link
Member

@trendzetter most websites don't have trailing slashes in the URL, and we decided to configure MEAN.js like that as well. Is there a reason you think it's a bad idea? #1224 doesn't have anything to do with trailing slashes, that has to do with the base tag in the HTML. Did you mistype the issue number?

@trendzetter
Copy link
Contributor

trendzetter commented May 3, 2016

@ilanbiala I linked to a comment, not the issue itself.

most websites don't have trailing slashes in the URL

Might be true but there is no error or even convention against it. I am using the trailing slashes for optional parameters in the url and it works fine (if I leave the slash checks out, which I think are overhead that was adopted too lightly)

@simison I linked to your comment (#1224 (comment)) saying that the rewriting of urls causes issues for writing your tests. Are you convinced by the argument pro-rewriting that @ilanbiala added or did you intend to send a thumbs up for my more critical look at this functionality?

@simison
Copy link
Member

simison commented May 3, 2016

@trendzetter well, suppose it's fine for a boilerplate to be opinionated and you can then modify it to your own needs, right?

@trendzetter
Copy link
Contributor

trendzetter commented May 3, 2016

@simison True but I think it's a distraction that is limiting unneeded and has the potential to hinder a lot, not me, it's removed in my fork.
meanjs should just create conventional url's (without trailing slashes) but actively rewriting is a bit overprotective I think. Anyway, I am willing to leave it like that but it's starting to appears already that this has it's own problems.

@simison
Copy link
Member

simison commented May 3, 2016

@trendzetter:

@simison I linked to your comment (#1224 (comment)) saying that the rewriting of urls causes issues for writing your tests. Are you convinced by the argument pro-rewriting that @ilanbiala added or did you intend to send a thumbs up for my more critical look at this functionality?

I like rewriting URLs as a straightforward way of handling trailing slashes (and I use it myself — although that shouldn't matter much).

edit; It's great to list issues followed by it to see if it's worth the hassle, but actually so far it seems every issue can be worked out.

@trendzetter
Copy link
Contributor

@simison

and I use it myself

I can imagine that you have those lines in your fork but are you really _using _them? It seems to me you would only use the code if you do strange things in your application that produces weird url's for no good reason.

@simison
Copy link
Member

simison commented May 3, 2016

@trendzetter our site is big enough to have all kinds of links pointing to it (broken links, outdated link formats, with and without trailing slashes, with and without #!). It's internet — better not to expect anything.

...so your app breaks if someone puts trailing slash on url? :-)

@trendzetter
Copy link
Contributor

My app does not break, maybe I should check how I handle that exactly, but the idea is that it you have a trailing slash on some of the routes in the app it means the optional parameter was not given and it returns content just as well.

@trendzetter
Copy link
Contributor

Just did some verification, and yes, some of the url's which are in fact invalid return a standard http error code 404 which is what I expect for an invalid url. I don't consider returning 404 for an invalid route "breaking"

@simison
Copy link
Member

simison commented May 3, 2016

I don't consider returning 404 for an invalid route "breaking"

Sure! That's engineering point of view. I'm more interested in user experience. User expects /pagename/ and /pagename to be the same.

@trendzetter
Copy link
Contributor

trendzetter commented May 3, 2016

Users do not get invalid routes by themselves. Developers navigate by writing directly in the URL.

@trendzetter
Copy link
Contributor

404 is intended to help the user and you could put a sitemap on the 404 page.

@trendzetter
Copy link
Contributor

trendzetter commented May 3, 2016

I noticed a bit of frustration with my comments on this issue, @simison I hope I didn't offend you, I am a bit surprised by your strong position on this. My only interest is having a respectful discussion that makes us all wiser.

@mleanos
Copy link
Member

mleanos commented May 3, 2016

If I can jump in here, the key point is that we're not re-writing the URL in this case to circumvent improperly architected links/routes (from within our framework).

We're doing this so the user's experience doesn't get interrupted when (as @simison already pointed out) we know the intention of the user when they type in /pagename/ vs /pagename; they must be treated the same. Again, to re-iterate what Mikael said, links & navigation could be coming from all sorts of places on the internet; we can't guarantee they will all be correctly formatted.

Lastly, I'm curious to know what your application's requirement is for you to need the /#!. Do your parameters not work without it?

@trendzetter
Copy link
Contributor

trendzetter commented May 4, 2016

@mleanos I understand the motivations to rewrite the url but I think they results are limiting and idea is a bit overprotective: the need to correct such typo's is better decided on a per application basis rather than in the framework. There is no rule, standard or even styleguide (I think) against trailing slashes. Concerning expectation from users how url should be, this is a bit projection by the developer mind I think. I never seen users fiddling directly in the URL in my IT support activities. It's webdevelopers who have a habit of doing that.

I will attempt to explain my use case with the optional parameters, it might well be that my implementation is not optimal and that I can work around the rewriting by structuring my application different.

I have a module, let's say /cars that lists the cars. I have added an optional parameter so the url is now:
/cars/list/ (all cars)
/cars/list/dodge (all cars with brand dodge)
/cars/list (404)

This is how it works now and it's ok I think, I developed it before the rewriting was activated and I had to comment out the rewriting to keep this application going after merge with the master meanjs.

@lirantal
Copy link
Member

@mleanos doesn't seem like we've got any action item so I'm closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants