Performance Improvement and IE7 Bug Fix #35

Closed
wants to merge 4 commits into
from

Projects

None yet

3 participants

@dvelyk
dvelyk commented Jun 25, 2011

This patch fixes the IE7 child selector bug (#21) by appending style elements directly to head before setting their cssText rather than adding them to a documentFragment and appending that to head.

I didn't change the insertBefore lastLink.nextSibling line otherwise, so that will need to be changed if the order of the elements in the DOM is important.

I've also modified applyMedia to bail if no styles change on resize. Resizing seems much smoother now.

@scottjehl scottjehl commented on the diff Jun 29, 2011
respond.src.js
@@ -213,8 +223,8 @@
xmlHttp = (function() {
var xmlhttpmethod = false,
attempts = [
- function(){ return new ActiveXObject("Microsoft.XMLHTTP") },
- function(){ return new XMLHttpRequest() }
+ function(){ return new XMLHttpRequest() },
+ function(){ return new ActiveXObject("Microsoft.XMLHTTP") }
@scottjehl
scottjehl Jun 29, 2011 Owner

Can you explain this change? The loop below goes in reverse, so the former order would hit the standards-based model first...

@dvelyk
dvelyk Jun 30, 2011

Hi! I'm afraid I can't really explain this change.. I hadn't intended this to be a part of my pull request, actually. I'm new to github ;) All I can tell you is that before I made the change, IE8 tabs were crashing (the "Can't use in IE8" issue -- #27), and switching the order put an end to that. At least, a couple days ago. Now I can't crash IE8 no matter what I do, and the order of these functions seems to make no difference at all. I don't think I've significantly changed anything in that time, so I'm not sure what the deal is.

Feel free to leave these lines out, or let me know if I need to remove the commit from my copy before you pull in the other changes. If IE8 starts crashing on me again I'll do some further experimenting to confirm if the XMLHttpRequest object is the culprit or not.

@chrislachance

Awesome! This solved my issues with IE7. Thanks!

@scottjehl
Owner

Hey @dvelyk - so a similar (maybe identical) fix landed recently that included this change, and you're right, it fixed a ton of issues in IE7 (though it did introduce an error when base elements are in play at initial load). Anyway, I'm closing this out, but I've added a shout-out to your idea in a comment in the script.
Thanks again!

@scottjehl scottjehl closed this Jan 27, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment