Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove an IE7 hack #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NattyNarwhal
Copy link
Contributor

No description provided.

@martinwoodward
Copy link
Member

While I'm not that bothered about IE7 support myself, I don't think we need to remove support for it now do we? What was the motivation for removing the code?

@NattyNarwhal
Copy link
Contributor Author

None of the supported platforms can run IE 7 (let alone a prelease version) - in addition, it's legacy code, which muddles understanding, wastes resources, and can get in the way of further improvements..

@ScottIsAFool
Copy link
Member

Is it confirmed that the hack is no longer necessary?

@NattyNarwhal
Copy link
Contributor Author

I did some quick tests and it seems fine to me. If there's a bug on the IE end, it should have been solved by IE 7 RTM at least.

@ScottIsAFool
Copy link
Member

That's fine then.

@kathweaver
Copy link
Contributor

From @ScottIsAFool on another one: We have moved to the dotnetfoundation's AppVeyor account and ScottH has closed his account which is what we used to be using. If @vhanla were to make an arbitrary commit to this branch and push it, it would force a rebuild on appveyor.

@RobDolinMS RobDolinMS added the needs-testing Fixes and features that need testing label Aug 30, 2016
@RobDolinMS
Copy link
Contributor

Merging this could be risky if a user has IE in a legacy compatibility mode.
It would be great to get more testing on this.

@dnfclas
Copy link

dnfclas commented Feb 14, 2017

@NattyNarwhal, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-testing Fixes and features that need testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants