From c62e4b2b4bf3cd22a02463902cf3deef53c6e189 Mon Sep 17 00:00:00 2001
From: Rob Ruana <rob@robruana.com>
Date: Mon, 31 Oct 2016 13:39:34 -0700
Subject: [PATCH] Closes #21055: Prevents ScrollSpy from clearing active item
 when Safari rubberbands

When the rubberband effect causes Safari to scroll past the top of the
page, the value of scrollTop becomes negative. If the offset of the first
ScrollSpy target is 0 - essentially if the target is at the top of the
page - then ScrollSpy should not clear the active item. Conceptually, the
first item should remain active when rubberbanding past the top of the
page.

This commit fixes issue #21055 by verifying the first scrollspy target is
not at the top of the page before clearing the active nav-item.
---
 js/src/scrollspy.js        |  2 +-
 js/tests/unit/scrollspy.js | 44 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/js/src/scrollspy.js b/js/src/scrollspy.js
index 604815f4e005..0756c1cc5a19 100644
--- a/js/src/scrollspy.js
+++ b/js/src/scrollspy.js
@@ -215,7 +215,7 @@ const ScrollSpy = (($) => {
         }
       }
 
-      if (this._activeTarget && scrollTop < this._offsets[0]) {
+      if (this._activeTarget && scrollTop < this._offsets[0] && this._offsets[0] > 0) {
         this._activeTarget = null
         this._clear()
         return
diff --git a/js/tests/unit/scrollspy.js b/js/tests/unit/scrollspy.js
index 97ddd16ece16..6bf597a75483 100644
--- a/js/tests/unit/scrollspy.js
+++ b/js/tests/unit/scrollspy.js
@@ -285,6 +285,50 @@ $(function () {
       .scrollTop(201)
   })
 
+  QUnit.test('should NOT clear selection if above the first section and first section is at the top', function (assert) {
+    assert.expect(4)
+    var done = assert.async()
+
+    var sectionHTML = '<div id="header" style="height: 500px;"></div>'
+        + '<nav id="navigation" class="navbar">'
+        + '<ul class="nav navbar-nav">'
+        + '<li><a id="one-link"   class="nav-link active" href="#one">One</a></li>'
+        + '<li><a id="two-link"   class="nav-link" href="#two">Two</a></li>'
+        + '<li><a id="three-link" class="nav-link" href="#three">Three</a></li>'
+        + '</ul>'
+        + '</nav>'
+    $(sectionHTML).appendTo('#qunit-fixture')
+
+    var negativeHeight = -10
+    var startOfSectionTwo = 101
+
+    var scrollspyHTML = '<div id="content" style="height: 200px; overflow-y: auto;">'
+        + '<div id="one" style="height: 100px;"/>'
+        + '<div id="two" style="height: 100px;"/>'
+        + '<div id="three" style="height: 100px;"/>'
+        + '<div id="spacer" style="height: 100px;"/>'
+        + '</div>'
+    var $scrollspy = $(scrollspyHTML).appendTo('#qunit-fixture')
+
+    $scrollspy
+      .bootstrapScrollspy({
+        target: '#navigation',
+        offset: $scrollspy.position().top
+      })
+      .one('scroll', function () {
+        assert.strictEqual($('.active').length, 1, '"active" class on only one element present')
+        assert.strictEqual($('.active').is('#two-link'), true, '"active" class on second section')
+        $scrollspy
+          .one('scroll', function () {
+            assert.strictEqual($('.active').length, 1, '"active" class on only one element present')
+            assert.strictEqual($('.active').is('#one-link'), true, '"active" class on first section')
+            done()
+          })
+          .scrollTop(negativeHeight)
+      })
+      .scrollTop(startOfSectionTwo)
+  })
+
   QUnit.test('should correctly select navigation element on backward scrolling when each target section height is 100%', function (assert) {
     assert.expect(5)
     var navbarHtml =