Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Extract doScrollCheck function into a variable #1192

Closed
wants to merge 1 commit into from

3 participants

Andrew Herron Rick Waldron Richard Gibson
Andrew Herron

I ship jQuery in a product I work on, and we recently discovered one of the places our product is used runs a JavaScript minification tool that breaks jQuery. It only renames the doScrollCheck call site, not the function declaration.

I do not have control over the minification tool so I was hoping this harmless change can be made in the core code instead.

This commit simply extracts the doScrollCheck function into a variable which allows the minification tool to pick up the function rename correctly.

Rick Waldron
Collaborator

Thanks for the contribution!

There isn't really a bug here... The problem you've encountered is not a shortcoming of jQuery's, but of the minification tool. Named function expressions whose identifier is bound to the scope created by the function expression has existed in the language since 1999, please file a ticket with the minification tool's author(s) and link them back to this issue. I'm going to close this for now, but we have it here if we need to revisit at a later date.

Rick Waldron rwaldron closed this
Andrew Herron

I'm aware of how crazy it is that this minification tool can't handle the named function expression. However it isn't used anywhere else in jQuery, and the entire function was removed in version 2 (it's a fix specific to IE8 and below) so I was hoping it could be merged despite the fact that it isn't a jQuery bug.

I have no control over the deployment of the minification tool, in fact I'm not even sure if there is a tool I can log a bug against (it's embedded in IBM Connections).

Rick Waldron
Collaborator

However it isn't used anywhere else in jQuery, and the entire function was removed in version 2 (it's a fix specific to IE8 and below)

Yes, I know... I removed it ;)

As for filing bugs, I'm going to ask that you at least try before assuming that jQuery wants to take the responsibility.

Richard Gibson
Collaborator

Immediately-invoked named function expressions are also used in callbacks.js. And not that it clinches the decision, but this particular pattern is not counted among the many perfectly valid expressions that nevertheless effect jshint warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 18 additions and 15 deletions.
  1. +18 −15 src/core.js
33 src/core.js
View
@@ -904,23 +904,26 @@ jQuery.ready.promise = function( obj ) {
} catch(e) {}
if ( top && top.doScroll ) {
- (function doScrollCheck() {
- if ( !jQuery.isReady ) {
-
- try {
- // Use the trick by Diego Perini
- // http://javascript.nwbox.com/IEContentLoaded/
- top.doScroll("left");
- } catch(e) {
- return setTimeout( doScrollCheck, 50 );
- }
+ (function() {
+ var doScrollCheck = function() {
+ if ( !jQuery.isReady ) {
+
+ try {
+ // Use the trick by Diego Perini
+ // http://javascript.nwbox.com/IEContentLoaded/
+ top.doScroll("left");
+ } catch(e) {
+ return setTimeout( doScrollCheck, 50 );
+ }
- // detach all dom ready events
- detach();
+ // detach all dom ready events
+ detach();
- // and execute any waiting functions
- jQuery.ready();
- }
+ // and execute any waiting functions
+ jQuery.ready();
+ }
+ };
+ doScrollCheck();
})();
}
}
Something went wrong with that request. Please try again.