chore($sniffer): Remove $sniffer.hashchange #9497
Conversation
The hashchange event is not supported only in ancient browsers like Android<2.2 and IE<8. Angular never really supported IE7 and in 1.3 where support for IE8 is dropped it makes even less sense to check for hashchange support.
|
Relevant caniuse: http://caniuse.com/#feat=hashchange |
| @@ -256,9 +256,7 @@ function Browser(window, document, $log, $sniffer) { | |||
| // html5 history api - popstate event | |||
| if ($sniffer.history) jqLite(window).on('popstate', fireUrlChange); | |||
| // hashchange event | |||
| if ($sniffer.hashchange) jqLite(window).on('hashchange', fireUrlChange); | |||
| // polling | |||
| else self.addPollFn(fireUrlChange); | |||
There was a problem hiding this comment.
There's quite a bit of code that's now not used that could be removed.
There was a problem hiding this comment.
This is still needed for ngCookies, isn't it?
There was a problem hiding this comment.
eventually I'd like to get this out of browser and move it to cookies, but now is not the time.
There was a problem hiding this comment.
url polling isn't needed for ngCookies, afaik
There was a problem hiding this comment.
@caitp not URL pooling, just $browser.addPollFn.
Obviously it could be moved to ngCookies but see what @IgorMinar said above.
There was a problem hiding this comment.
this CL doesn't get rid of addPollFn though. (unless I'm not seeing something in the diff?)
Maybe I'm just misunderstanding the conversation, I thought you were saying "don't remove this, ngCookies needs it", but the thing ngCookies needs is not removed
There was a problem hiding this comment.
Yes. @realityking suggested more code can be removed, referring to $browser.addPollFn. I noted that it's still used in ngCookies co cannot be removed; @IgorMinar said it can be moved to ngCookies but not now.
There was a problem hiding this comment.
IOW This in This is still needed for ngCookies referred to $browser.addPollFn, not the part removed here; sorry I wasn't more clear!
There was a problem hiding this comment.
My bad, I had missed that itt was used in ngCookies.
|
lgtm |
|
Landed at 1beebee |
The hashchange event is not supported only in ancient browsers like Android<2.2
and IE<8. Angular never really supported IE7 and in 1.3 where support for IE8
is dropped it makes even less sense to check for hashchange support.