Minor bugfixes and code optimization #49
Conversation
Merge with original repository
This change implements next features: 1) Deal with "access is denied" error in IE 7/8 if script is using in local files (demo example doesn't work in IE 7/8, only IE6). Thanks to CSSUtilities plugin (http://www.brothercake.com/scripts/cssutilities) for information inside source code 2) Code optimization: using ActiveXObject instead of XMLHttpRequest for IE 7/8 allowed to skip browser check and use closure without "new" keyword which is equal to all other browsers 3) getXMLHttpRequest was highly optimized and moved to xhr() method due its small size
|
Only after commit I found that problem with "new" keyword for onreadystatechange() closure in IE 6/7 was fixed because I moved onreadystatechange() declaration before send() (what is logically correct) and not because I used ActiveXObject for IE 7/8 (which isn't connect with IE6). So in case when onreadystatechange() is initializing before using (sending request) "new" keyword is not required for old versions of IE. Tests confirm this |
RegExp is using for removing comments now. This works faster (~5-15%) and needs less code
|
I think I found tool which You are using for minification. Is this a UglifyJS 2 (http://gpbmike.github.io/refresh-sf/)? |
Next toolkit was used: http://gpbmike.github.io/refresh-sf/
1) Fixed unused and duplicating variables in xhr() arguments 2) Code was minified (unnecessary function and condition were removed) 3) Code style fixes
|
But you didn't merge all my changes. There is also 774196f commit with several code optimizations |
|
You are correct, I didn't realized you pushed more commits after I'd already pulled down the branch for testing. Please update your pull request from master so I can test/merge again. |
|
Side note, could you please install/use GruntJS to run the grunt task that will lint and build the final file? |
| @@ -56,7 +52,7 @@ | |||
| }, | |||
|
|
|||
| matchCSS = function ( sheetCSS, link ) { // collect all of the rules from the xhr response texts and match them to a pattern | |||
| var clean = removeComments( removeMediaQueries(sheetCSS) ), | |||
| var clean = removeMediaQueries( sheetCSS ).replace(/\/\*[\s\S]*?\*\//g, ''), // remove MediaQueries and comments | |||
There was a problem hiding this comment.
This is a nice optimization! :)
|
@alray, figured out how to get these in and rerun the build. Will bump the version and release. |
I tested this in IE 6-8 (native), IE6-10 (compatibility mode), Firefox 2.0, Opera 9.52 and everything seems to work