From 1776e3d293825fe2788cc7dfcd26ec8fd03cc4fb Mon Sep 17 00:00:00 2001 From: Dave Methvin Date: Fri, 5 Aug 2016 17:45:13 -0400 Subject: [PATCH 1/2] Event: Warn about late use of $(window).on("load"...) Fixes #200 --- src/event.js | 10 ++++++++++ test/event-lateload.html | 37 +++++++++++++++++++++++++++++++++++++ test/event.js | 9 +++++++++ warnings.md | 6 ++++++ 4 files changed, 62 insertions(+) create mode 100644 test/event-lateload.html diff --git a/src/event.js b/src/event.js index bdb06479..d7bce4f4 100644 --- a/src/event.js +++ b/src/event.js @@ -1,4 +1,5 @@ var oldLoad = jQuery.fn.load, + oldEventAdd = jQuery.event.add, originalFix = jQuery.event.fix; jQuery.event.props = []; @@ -35,6 +36,15 @@ jQuery.event.fix = function( originalEvent ) { return fixHook && fixHook.filter ? fixHook.filter( event, originalEvent ) : event; }; +jQuery.event.add = function( elem, types ) { + + // This misses the multiple-types case but that seems awfully rare + if ( elem === window && types === "load" && document.readyState === "complete" ) { + migrateWarn( "$(window).on('load'...) called after load event occurred" ); + } + return oldEventAdd.apply( this, arguments ); +}; + jQuery.each( [ "load", "unload", "error" ], function( _, name ) { jQuery.fn[ name ] = function() { diff --git a/test/event-lateload.html b/test/event-lateload.html new file mode 100644 index 00000000..21b40a76 --- /dev/null +++ b/test/event-lateload.html @@ -0,0 +1,37 @@ + + + + + jQuery Migrate late load event binding test + + + + + + + + + +

jQuery Migrate late load event binding test

+ + diff --git a/test/event.js b/test/event.js index 9c5f2035..e5b8a5e7 100644 --- a/test/event.js +++ b/test/event.js @@ -145,3 +145,12 @@ TestManager.runIframeTest( "jQuery.event.fixHooks", "event-fixHooks.html", assert.equal( jQuery.migrateWarnings.length, 2, "warnings: " + JSON.stringify( jQuery.migrateWarnings ) ); } ); + +TestManager.runIframeTest( "Load within a ready handler", "event-lateload.html", + function( assert, jQuery, window, document, log ) { + assert.expect( 2 ); + + assert.equal( jQuery.migrateWarnings.length, 1, "warnings: " + + JSON.stringify( jQuery.migrateWarnings ) ); + assert.ok( /load/.test( jQuery.migrateWarnings[ 0 ] ), "message ok" ); + } ); diff --git a/warnings.md b/warnings.md index ce989269..c5795d42 100644 --- a/warnings.md +++ b/warnings.md @@ -193,3 +193,9 @@ See jQuery-ui [commit](https://github.com/jquery/jquery-ui/commit/c0093b599fcd58 **Cause:** The code on the page has used the `jQuery.event.props` or `jQuery.event.fixHooks` data structures. These were used in previous versions to affect the properties that are copied from the native event to the jQuery event each time an event is delivered, but they had the potential to create performance issues. Versions of jQuery Mobile before 1.5 make use of this API and require jQuery Migrate to run properly. **Solution:** The most popular use of these data structures are to add properties for touch or pointer events, and those properties are now supported by default with a newer high-performance approach in jQuery 3.0 that only retrieves the properties on first access. If you are using jQuery Mobile, check the [jquerymobile.com](https://jquerymobile.com) site for updates. If you are using plugins such as [pointerTouch](https://github.com/timmywil/jquery.event.pointertouch) or [touchHooks](https://github.com/aarongloege/jquery.touchHooks), simply remove them as they are no longer needed. + +### JQMIGRATE: $(window).on('load'...) called after load event occurred + +**Cause:** The calling code has attempted to attach a `load` event to `window` after the page has already loaded. That means the handler will never run and so is probably not what the caller intended. This can occur when the event attachment is made too late, for example, in a jQuery ready handler. It can also occur when a file is loaded dynamically with jQuery after the page has loaded, for example using the `$.getScript()` method. + +**Solution:** If a function `fn` does not actually depend on all page assets being fully loaded, switch to a ready handler `$( fn )` which runs earlier and will aways run `fn` even if the script that contains the code loads long after the page has fully loaded. If `fn` actually does depend on the script being fully loaded, check `document.readyState`. If the value is `"complete"` run the function immediately, otherwise use `$(window).on( "load", fn )`. From 7f1ff0e6e18ac0386b43bb61bad51531bdf000b4 Mon Sep 17 00:00:00 2001 From: Dave Methvin Date: Sun, 25 Sep 2016 21:17:41 -0400 Subject: [PATCH 2/2] squash! Use jQuery instead of $ in messages --- src/event.js | 2 +- warnings.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/event.js b/src/event.js index d7bce4f4..238c6257 100644 --- a/src/event.js +++ b/src/event.js @@ -40,7 +40,7 @@ jQuery.event.add = function( elem, types ) { // This misses the multiple-types case but that seems awfully rare if ( elem === window && types === "load" && document.readyState === "complete" ) { - migrateWarn( "$(window).on('load'...) called after load event occurred" ); + migrateWarn( "jQuery(window).on('load'...) called after load event occurred" ); } return oldEventAdd.apply( this, arguments ); }; diff --git a/warnings.md b/warnings.md index c5795d42..79232d23 100644 --- a/warnings.md +++ b/warnings.md @@ -194,7 +194,7 @@ See jQuery-ui [commit](https://github.com/jquery/jquery-ui/commit/c0093b599fcd58 **Solution:** The most popular use of these data structures are to add properties for touch or pointer events, and those properties are now supported by default with a newer high-performance approach in jQuery 3.0 that only retrieves the properties on first access. If you are using jQuery Mobile, check the [jquerymobile.com](https://jquerymobile.com) site for updates. If you are using plugins such as [pointerTouch](https://github.com/timmywil/jquery.event.pointertouch) or [touchHooks](https://github.com/aarongloege/jquery.touchHooks), simply remove them as they are no longer needed. -### JQMIGRATE: $(window).on('load'...) called after load event occurred +### JQMIGRATE: jQuery(window).on('load'...) called after load event occurred **Cause:** The calling code has attempted to attach a `load` event to `window` after the page has already loaded. That means the handler will never run and so is probably not what the caller intended. This can occur when the event attachment is made too late, for example, in a jQuery ready handler. It can also occur when a file is loaded dynamically with jQuery after the page has loaded, for example using the `$.getScript()` method.