Offset: add tests for hidden elements + scroll

timmywil · timmywil · commit a0a5c0be2def · 2015-06-16T13:24:12.000-04:00
- Also add comments to hidden/disconnected tests noting
  this is to ensure consistency between branches
diff --git a/src/offset.js b/src/offset.js
@@ -108,14 +108,20 @@ jQuery.fn.extend({
 		}
 
 		rect = elem.getBoundingClientRect();
-		win = getWindow( doc );
 
-		return {
-			top: rect.top  + ( win.pageYOffset || docElem.scrollTop ) -
-				( docElem.clientTop  || 0 ),
-			left: rect.left + ( win.pageXOffset || docElem.scrollLeft ) -
-				( docElem.clientLeft || 0 )
-		};
+		if ( rect.width || rect.height || elem.getClientRects().length ) {
+			win = getWindow( doc );
+
+			return {
+				top: rect.top  + ( win.pageYOffset || docElem.scrollTop ) -
+					( docElem.clientTop  || 0 ),
+				left: rect.left + ( win.pageXOffset || docElem.scrollLeft ) -
+					( docElem.clientLeft || 0 )
+			};
+		}
+
+		// Return zeros for hidden elements
+		return rect;
 	},
 
 	position: function() {
diff --git a/test/data/offset/scroll.html b/test/data/offset/scroll.html
@@ -11,6 +11,7 @@
 				#scroll-1-1 { top: 1px; left: 1px; }
 					#scroll-1-1-1 { top: 1px; left: 1px; }
 			#forceScroll { width: 5000px; height: 5000px; }
+			#hidden { display: none; }
 			#marker { position: absolute; border: 2px solid #000; width: 50px; height: 50px; background: #ccc; }
 		</style>
 		<script src="../../jquery.js"></script>
@@ -32,6 +33,7 @@
 				<div id="scroll-1-1-1" class="scroll"></div>
 			</div>
 		</div>
+		<div id="hidden"></div>
 		<div id="forceScroll"></div>
 		<div id="marker"></div>
 		<p class="instructions">Click the white box to move the marker to it.</p>
diff --git a/test/unit/offset.js b/test/unit/offset.js
@@ -53,6 +53,9 @@ test("disconnected element", function() {
 
 	var result = jQuery( document.createElement( "div" ) ).offset();
 
+	// These tests are solely for master/compat consistency
+	// Retrieving offset on disconnected/hidden elements is not officially
+	// valid input, but will return zeros for back-compat
 	equal( result.top, 0, "Retrieving offset on disconnected elements returns zeros (gh-2310)" );
 	equal( result.left, 0, "Retrieving offset on disconnected elements returns zeros (gh-2310)" );
 });
@@ -65,6 +68,9 @@ test("hidden (display: none) element", function() {
 
 	node.remove();
 
+	// These tests are solely for master/compat consistency
+	// Retrieving offset on disconnected/hidden elements is not officially
+	// valid input, but will return zeros for back-compat
 	equal( result.top, 0, "Retrieving offset on hidden elements returns zeros (gh-2310)" );
 	equal( result.left, 0, "Retrieving offset on hidden elements returns zeros (gh-2310)" );
 });
@@ -406,7 +412,7 @@ testIframe("offset/table", "table", function( $ ) {
 });
 
 testIframe("offset/scroll", "scroll", function( $, win ) {
-	expect(28);
+	expect( 30 );
 
 	// If we're going to bastardize the tests, let's just DO it
 	var ie = /msie 8/i.test( navigator.userAgent );
@@ -425,6 +431,12 @@ testIframe("offset/scroll", "scroll", function( $, win ) {
 	}
 	equal( $("#scroll-1-1").offset().left, 11, "jQuery('#scroll-1-1').offset().left" );
 
+	// These tests are solely for master/compat consistency
+	// Retrieving offset on disconnected/hidden elements is not officially
+	// valid input, but will return zeros for back-compat
+	equal( $("#hidden").offset().top, 0, "Hidden elements do not subtract scroll" );
+	equal( $("#hidden").offset().left, 0, "Hidden elements do not subtract scroll" );
+
 	// scroll offset tests .scrollTop/Left
 	equal( $("#scroll-1").scrollTop(), 5, "jQuery('#scroll-1').scrollTop()" );
 	equal( $("#scroll-1").scrollLeft(), 5, "jQuery('#scroll-1').scrollLeft()" );
