Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Fix hashchange event on IE7 #361

Closed
wants to merge 2 commits into from

Conversation

vojtajina
Copy link
Contributor

Stupid IE7 returns true for ('onhashchange' in window), but does not support hashchange event.

Fix hashchange event on IE7

Stupid IE7 returns true for ('onhashchange' in window), but does not support hashchange event.

Instead of using sniffing tricks like this:

window.setAttribute('onhashchange', 'return;');
typeof window.onhashchange == 'function';

I decided to check msie version. That's why had to remove two unit tests:

  • should use $browser poller to detect url changes when onhashchange event is unsupported
  • should use onhashchange events to detect url changes when supported by browser

I hope that's all right - we don't care whether event or poller is used. We just check whether the $browser.onHashChange event works, running the tests in all browsers.

@esprehn
Copy link
Contributor

esprehn commented Jun 1, 2011

Pretty sure this is not IE7, but IE8 in compatibility mode that lies about the property.

I'd leave the unit tests. I think it's bad to have to run the tests in every browser to execute the two code paths properly. You should be able to have Chrome only attached to jstd when developing and not worry about accidentally breaking poller. Of course Igor's opinion on this is probably more important. :)

@vojtajina
Copy link
Contributor Author

Hey, you are probably right - I don't have real IE7.
I use IE8 and both IE8 compatibility mode and IE7 mode lie about the property.

I would leave these tests, but now, the behavior relies on msie property, so I would have to mutate this global property to get the test passed on IE7.

Will discuss that with Igor, thanks for the comment...

@IgorMinar
Copy link
Contributor

as discussed today - please change this to use compatibility mode detection just like modernizr

as far as elliott's comment about two execution paths goes - this is browser.js, it is messy, ugly and dirty just like the DOM api, so I think it's ok that we have the branch here.

vojtajina added 2 commits June 1, 2011 22:54
Stupid IE8 in compatibility mode or in IE7 mode returns true for `('onhashchange' in window)`, but does not support hashchange event.

Closes angular#353
@IgorMinar
Copy link
Contributor

angular.js: master commits aa64d37...dad2603 - http://bit.ly/ldzsiA

@IgorMinar IgorMinar closed this Jun 2, 2011
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants