From d03d2e9f26f85366ad2e91b9e2c76a249d7bf7be Mon Sep 17 00:00:00 2001 From: Xavi Date: Sun, 9 Jan 2011 19:11:05 -0500 Subject: [PATCH 1/6] Bug 7931; Fixed bug that caused scrollTop and scrollLeft setters to return null when called on an empty jquery object --- src/offset.js | 28 ++++++++++++++-------------- test/unit/offset.js | 8 +++++++- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/offset.js b/src/offset.js index 2040c9d8..d53a8813 100644 --- a/src/offset.js +++ b/src/offset.js @@ -261,13 +261,9 @@ jQuery.each( ["Left", "Top"], function( i, name ) { var method = "scroll" + name; jQuery.fn[ method ] = function(val) { - var elem = this[0], win; + var elem, win; - if ( !elem ) { - return null; - } - - if ( val !== undefined ) { + if ( val != undefined ) { // Set the scroll offset return this.each(function() { win = getWindow( this ); @@ -282,15 +278,19 @@ jQuery.each( ["Left", "Top"], function( i, name ) { this[ method ] = val; } }); - } else { - win = getWindow( elem ); - - // Return the scroll offset - return win ? ("pageXOffset" in win) ? win[ i ? "pageYOffset" : "pageXOffset" ] : - jQuery.support.boxModel && win.document.documentElement[ method ] || - win.document.body[ method ] : - elem[ method ]; } + + elem = this[0]; + if( !elem ) { + return null + } + + win = getWindow( elem ); + // Return the scroll offset + return win ? ("pageXOffset" in win) ? win[ i ? "pageYOffset" : "pageXOffset" ] : + jQuery.support.boxModel && win.document.documentElement[ method ] || + win.document.body[ method ] : + elem[ method ]; }; }); diff --git a/test/unit/offset.js b/test/unit/offset.js index cfa14449..66676def 100644 --- a/test/unit/offset.js +++ b/test/unit/offset.js @@ -333,7 +333,7 @@ testoffset("table", function( jQuery ) { }); testoffset("scroll", function( jQuery, win ) { - expect(16); + expect(20); var ie = jQuery.browser.msie && parseInt( jQuery.browser.version, 10 ) < 8; @@ -379,6 +379,12 @@ testoffset("scroll", function( jQuery, win ) { equals( jQuery(window).scrollLeft(), 0, "jQuery(window).scrollLeft() other window" ); equals( jQuery(document).scrollTop(), 0, "jQuery(window).scrollTop() other document" ); equals( jQuery(document).scrollLeft(), 0, "jQuery(window).scrollLeft() other document" ); + + // Tests scrollTop/Left with empty jquery objects + ok( jQuery().scrollTop(100) != null, "jQuery().scrollTop(100) testing setter on empty jquery object" ); + ok( jQuery().scrollLeft(100) != null, "jQuery().scrollLeft(100) testing setter on empty jquery object" ); + ok( jQuery().scrollTop() === null, "jQuery().scrollTop(100) testing setter on empty jquery object" ); + ok( jQuery().scrollLeft() === null, "jQuery().scrollLeft(100) testing setter on empty jquery object" ); }); testoffset("body", function( jQuery ) { From 628bacc3ce26a7a0e0462c2a2e0d764edf859c97 Mon Sep 17 00:00:00 2001 From: Xavi Date: Sun, 9 Jan 2011 20:12:29 -0500 Subject: [PATCH 2/6] Bug 7931; Added missing semicolon and replaced '!=' with '!==' to allow null through --- src/offset.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/offset.js b/src/offset.js index d53a8813..02f067eb 100644 --- a/src/offset.js +++ b/src/offset.js @@ -263,7 +263,7 @@ jQuery.each( ["Left", "Top"], function( i, name ) { jQuery.fn[ method ] = function(val) { var elem, win; - if ( val != undefined ) { + if ( val !== undefined ) { // Set the scroll offset return this.each(function() { win = getWindow( this ); @@ -282,7 +282,7 @@ jQuery.each( ["Left", "Top"], function( i, name ) { elem = this[0]; if( !elem ) { - return null + return null; } win = getWindow( elem ); From 8d28f41f669168ebd436599f8b187c783fec4ba9 Mon Sep 17 00:00:00 2001 From: Xavi Date: Sun, 9 Jan 2011 20:34:15 -0500 Subject: [PATCH 3/6] Bug 7931; Replaced with and --- test/unit/offset.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/offset.js b/test/unit/offset.js index 66676def..20bd7079 100644 --- a/test/unit/offset.js +++ b/test/unit/offset.js @@ -381,10 +381,10 @@ testoffset("scroll", function( jQuery, win ) { equals( jQuery(document).scrollLeft(), 0, "jQuery(window).scrollLeft() other document" ); // Tests scrollTop/Left with empty jquery objects - ok( jQuery().scrollTop(100) != null, "jQuery().scrollTop(100) testing setter on empty jquery object" ); - ok( jQuery().scrollLeft(100) != null, "jQuery().scrollLeft(100) testing setter on empty jquery object" ); - ok( jQuery().scrollTop() === null, "jQuery().scrollTop(100) testing setter on empty jquery object" ); - ok( jQuery().scrollLeft() === null, "jQuery().scrollLeft(100) testing setter on empty jquery object" ); + notEqual( jQuery().scrollTop(100), null, "jQuery().scrollTop(100) testing setter on empty jquery object" ); + notEqual( jQuery().scrollLeft(100), null, "jQuery().scrollLeft(100) testing setter on empty jquery object" ); + strictEqual( jQuery().scrollTop(), null, "jQuery().scrollTop(100) testing setter on empty jquery object" ); + strictEqual( jQuery().scrollLeft(), null, "jQuery().scrollLeft(100) testing setter on empty jquery object" ); }); testoffset("body", function( jQuery ) { From bed64e65cc658409961a7127b715643390919e6c Mon Sep 17 00:00:00 2001 From: Xavi Date: Sun, 9 Jan 2011 20:39:23 -0500 Subject: [PATCH 4/6] Bug 7931; Added unit tests for scrollTop/Left. --- test/unit/offset.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/offset.js b/test/unit/offset.js index 20bd7079..69bf6d5d 100644 --- a/test/unit/offset.js +++ b/test/unit/offset.js @@ -333,7 +333,7 @@ testoffset("table", function( jQuery ) { }); testoffset("scroll", function( jQuery, win ) { - expect(20); + expect(22); var ie = jQuery.browser.msie && parseInt( jQuery.browser.version, 10 ) < 8; @@ -383,6 +383,8 @@ testoffset("scroll", function( jQuery, win ) { // Tests scrollTop/Left with empty jquery objects notEqual( jQuery().scrollTop(100), null, "jQuery().scrollTop(100) testing setter on empty jquery object" ); notEqual( jQuery().scrollLeft(100), null, "jQuery().scrollLeft(100) testing setter on empty jquery object" ); + notEqual( jQuery().scrollTop(null), null, "jQuery().scrollTop(null) testing setter on empty jquery object" ); + notEqual( jQuery().scrollLeft(null), null, "jQuery().scrollLeft(null) testing setter on empty jquery object" ); strictEqual( jQuery().scrollTop(), null, "jQuery().scrollTop(100) testing setter on empty jquery object" ); strictEqual( jQuery().scrollLeft(), null, "jQuery().scrollLeft(100) testing setter on empty jquery object" ); }); From b78e3fc39f96d63a60ea66e3d066815626e633d8 Mon Sep 17 00:00:00 2001 From: Xavi Date: Sun, 9 Jan 2011 20:51:20 -0500 Subject: [PATCH 5/6] Bug 7931; Inverted logic in scrollTop/Left (i.e. made --- src/offset.js | 52 +++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/offset.js b/src/offset.js index 02f067eb..660efa23 100644 --- a/src/offset.js +++ b/src/offset.js @@ -263,34 +263,34 @@ jQuery.each( ["Left", "Top"], function( i, name ) { jQuery.fn[ method ] = function(val) { var elem, win; - if ( val !== undefined ) { - // Set the scroll offset - return this.each(function() { - win = getWindow( this ); - - if ( win ) { - win.scrollTo( - !i ? val : jQuery(win).scrollLeft(), - i ? val : jQuery(win).scrollTop() - ); - - } else { - this[ method ] = val; - } - }); + if(val === undefined) { + elem = this[0]; + if( !elem ) { + return null; + } + + win = getWindow( elem ); + // Return the scroll offset + return win ? ("pageXOffset" in win) ? win[ i ? "pageYOffset" : "pageXOffset" ] : + jQuery.support.boxModel && win.document.documentElement[ method ] || + win.document.body[ method ] : + elem[ method ]; } - elem = this[0]; - if( !elem ) { - return null; - } - - win = getWindow( elem ); - // Return the scroll offset - return win ? ("pageXOffset" in win) ? win[ i ? "pageYOffset" : "pageXOffset" ] : - jQuery.support.boxModel && win.document.documentElement[ method ] || - win.document.body[ method ] : - elem[ method ]; + // Set the scroll offset + return this.each(function() { + win = getWindow( this ); + + if ( win ) { + win.scrollTo( + !i ? val : jQuery(win).scrollLeft(), + i ? val : jQuery(win).scrollTop() + ); + + } else { + this[ method ] = val; + } + }); }; }); From 135a384cf3e4cc3671de4cfcced20f294a5667a8 Mon Sep 17 00:00:00 2001 From: Xavi Date: Tue, 18 Jan 2011 12:40:07 -0500 Subject: [PATCH 6/6] Bug 7931; cleaned up white space in accordance to style guide --- src/offset.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/offset.js b/src/offset.js index 660efa23..8696c205 100644 --- a/src/offset.js +++ b/src/offset.js @@ -260,15 +260,15 @@ jQuery.fn.extend({ jQuery.each( ["Left", "Top"], function( i, name ) { var method = "scroll" + name; - jQuery.fn[ method ] = function(val) { + jQuery.fn[ method ] = function( val ) { var elem, win; - if(val === undefined) { - elem = this[0]; + if( val === undefined ) { + elem = this[ 0 ]; if( !elem ) { return null; } - + win = getWindow( elem ); // Return the scroll offset return win ? ("pageXOffset" in win) ? win[ i ? "pageYOffset" : "pageXOffset" ] : @@ -276,17 +276,16 @@ jQuery.each( ["Left", "Top"], function( i, name ) { win.document.body[ method ] : elem[ method ]; } - + // Set the scroll offset return this.each(function() { win = getWindow( this ); if ( win ) { win.scrollTo( - !i ? val : jQuery(win).scrollLeft(), - i ? val : jQuery(win).scrollTop() + !i ? val : jQuery( win ).scrollLeft(), + i ? val : jQuery( win ).scrollTop() ); - } else { this[ method ] = val; }