From b65cd87d7b5bd50063951d5107a1c28e77175194 Mon Sep 17 00:00:00 2001
From: joakin <joaquin@chimeces.com>
Date: Thu, 10 Mar 2016 13:10:30 +0000
Subject: [PATCH] Fix mw.viewport.isElementInViewport with scrolled page

With a scrolled page the function would return false since it assumed that
getBoundingClientRect returns the offset when in reality it returns the offset
relative to the window viewport.

This implementation keeps the ability to specify custom viewport rects. If we
wanted to just rely on the window viewport the implementation could be changed
with getBoundingClientRect to be much simpler.

Bug: T129466
Change-Id: I56142fbd1177fe47171a1874f73b7a8359c282ee
---
 resources/src/mediawiki/mediawiki.viewport.js | 21 +++++++++++++----
 .../mediawiki/mediawiki.viewport.test.js      | 23 +++++++++++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/resources/src/mediawiki/mediawiki.viewport.js b/resources/src/mediawiki/mediawiki.viewport.js
index aa9dd0581f..6396331f90 100644
--- a/resources/src/mediawiki/mediawiki.viewport.js
+++ b/resources/src/mediawiki/mediawiki.viewport.js
@@ -50,14 +50,25 @@
 		 * @return {boolean}
 		 */
 		isElementInViewport: function ( el, rectangle ) {
-			var elRect = el.getBoundingClientRect(),
+			var $el = $( el ),
+				offset = $el.offset(),
+				rect = {
+					height: $el.height(),
+					width: $el.width(),
+					top: offset.top,
+					left: offset.left
+				},
 				viewport = rectangle || this.makeViewportFromWindow();
 
 			return (
-				( viewport.bottom >= elRect.top ) &&
-				( viewport.right >= elRect.left ) &&
-				( viewport.top <= elRect.top + elRect.height ) &&
-				( viewport.left <= elRect.left + elRect.width )
+				// Top border must be above viewport's bottom
+				( viewport.bottom >= rect.top ) &&
+				// Left border must be before viewport's right border
+				( viewport.right >= rect.left ) &&
+				// Bottom border must be below viewport's top
+				( viewport.top <= rect.top + rect.height ) &&
+				// Right border must be after viewport's left border
+				( viewport.left <= rect.left + rect.width )
 			);
 		},
 
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js
index 61391d8bc8..f404294215 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js
@@ -60,6 +60,29 @@
 			'It should default to the window object if no viewport is given' );
 	} );
 
+	QUnit.test( 'isElementInViewport with scrolled page', 1, function ( assert ) {
+		var viewport = {
+				top: 2000,
+				left: 0,
+				right: 1000,
+				bottom: 2500
+			},
+			el = $( '<div />' )
+				.appendTo( '#qunit-fixture' )
+				.width( 20 )
+				.height( 20 )
+				.offset( {
+					top: 2300,
+					left: 20
+				} )
+				.get( 0 );
+		window.scrollTo( viewport.left, viewport.top );
+		assert.ok( mw.viewport.isElementInViewport( el, viewport ),
+			'It should return true when the element is fully enclosed in the ' +
+			'viewport even when the page is scrolled down' );
+		window.scrollTo( 0, 0 );
+	} );
+
 	QUnit.test( 'isElementCloseToViewport', 3, function ( assert ) {
 		var
 			viewport = {
-- 
2.20.1