From 487513ff03b3d63ad3b3fe641dae2a3882d5ded5 Mon Sep 17 00:00:00 2001
From: David Bailey <techdavid@users.noreply.github.com>
Date: Mon, 28 Aug 2017 16:21:04 +0100
Subject: [PATCH 1/5] Add failing test

When the body does not overflow (achieved by hiding the QUnit container), it should not be given a margin.
---
 js/tests/unit/modal.js | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/js/tests/unit/modal.js b/js/tests/unit/modal.js
index 5b265df158..3611dfdf11 100644
--- a/js/tests/unit/modal.js
+++ b/js/tests/unit/modal.js
@@ -391,6 +391,25 @@ $(function () {
       .bootstrapModal('show')
   })
 
+  QUnit.test('should not adjust the inline body padding when it does not overflow', function (assert) {
+    assert.expect(1)
+    var done = assert.async()
+    var $body = $(document.body)
+    var originalPadding = $body.css('padding-right')
+
+    $('#qunit-container').hide()
+    $('<div id="modal-test"/>')
+      .on('shown.bs.modal', function () {
+        var currentPadding = $body.css('padding-right')
+        assert.strictEqual(currentPadding, originalPadding, 'body padding should not be adjusted')
+        $(this).bootstrapModal('hide')
+
+        $('#qunit-container').show()
+        done()
+      })
+      .bootstrapModal('show')
+  })
+
   QUnit.test('should adjust the inline padding of fixed elements when opening and restore when closing', function (assert) {
     assert.expect(2)
     var done = assert.async()

From a4fff7c38397fa8fb59891cc94807be020ac37ec Mon Sep 17 00:00:00 2001
From: David Bailey <techdavid@users.noreply.github.com>
Date: Mon, 28 Aug 2017 18:35:47 +0100
Subject: [PATCH 2/5] Fix unwanted body padding when a modal opens

Prevents the test from failing
---
 js/src/modal.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/js/src/modal.js b/js/src/modal.js
index ab73230c8f..a88c144447 100644
--- a/js/src/modal.js
+++ b/js/src/modal.js
@@ -426,7 +426,7 @@ const Modal = (($) => {
     }
 
     _checkScrollbar() {
-      this._isBodyOverflowing = document.body.clientWidth < window.innerWidth
+      this._isBodyOverflowing = document.body.offsetWidth < window.innerWidth
       this._scrollbarWidth = this._getScrollbarWidth()
     }
 

From 2725acc9e576059e2dbb3e4722dd5beef84e9215 Mon Sep 17 00:00:00 2001
From: David Bailey <techdavid@users.noreply.github.com>
Date: Mon, 28 Aug 2017 18:44:56 +0100
Subject: [PATCH 3/5] Use jQuery outerWidth instead of offsetWidth

---
 js/src/modal.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/js/src/modal.js b/js/src/modal.js
index a88c144447..c8c7e3d217 100644
--- a/js/src/modal.js
+++ b/js/src/modal.js
@@ -426,7 +426,7 @@ const Modal = (($) => {
     }
 
     _checkScrollbar() {
-      this._isBodyOverflowing = document.body.offsetWidth < window.innerWidth
+      this._isBodyOverflowing = $('body').outerWidth(true) < window.innerWidth
       this._scrollbarWidth = this._getScrollbarWidth()
     }
 

From 3f2a8db4711e420b48b7a5b4c36f158a1b9eabab Mon Sep 17 00:00:00 2001
From: David Bailey <techdavid@users.noreply.github.com>
Date: Mon, 28 Aug 2017 18:55:45 +0100
Subject: [PATCH 4/5] Use getBoundingClientRect instead of jQuery

---
 js/src/modal.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/js/src/modal.js b/js/src/modal.js
index c8c7e3d217..0306b4e873 100644
--- a/js/src/modal.js
+++ b/js/src/modal.js
@@ -426,7 +426,8 @@ const Modal = (($) => {
     }
 
     _checkScrollbar() {
-      this._isBodyOverflowing = $('body').outerWidth(true) < window.innerWidth
+      const rect = document.body.getBoundingClientRect()
+      this._isBodyOverflowing = rect.left + rect.right < window.innerWidth
       this._scrollbarWidth = this._getScrollbarWidth()
     }
 

From 2622a015f2d38673e3af92a3031ae8b2f2a9ce0f Mon Sep 17 00:00:00 2001
From: David Bailey <techdavid@users.noreply.github.com>
Date: Tue, 29 Aug 2017 12:01:41 +0100
Subject: [PATCH 5/5] Fix unit tests in PhantomJS

Use a virtual scrollbar as this is simpler than having a real one (overflow: scroll doesn't seem to work in Phantom), and disable it for the new test.

One test has also been altered to prevent erroneous fails when other inline styles are added to the body (e.g. overflow).
---
 js/tests/unit/modal.js | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/js/tests/unit/modal.js b/js/tests/unit/modal.js
index 3611dfdf11..735dc4f0c8 100644
--- a/js/tests/unit/modal.js
+++ b/js/tests/unit/modal.js
@@ -21,6 +21,8 @@ $(function () {
         document.body.removeChild(scrollDiv)
         return scrollbarWidth
       }
+      // Simulate scrollbars in PhantomJS
+      $('html').css('padding-right', '16px')
     },
     beforeEach: function () {
       // Run all tests in noConflict mode -- it's the only way to ensure that the plugin works in noConflict mode
@@ -397,14 +399,19 @@ $(function () {
     var $body = $(document.body)
     var originalPadding = $body.css('padding-right')
 
-    $('#qunit-container').hide()
+    // Hide scrollbars to prevent the body overflowing
+    $body.css('overflow', 'hidden')        // real scrollbar (for in-browser testing)
+    $('html').css('padding-right', '0px')  // simulated scrollbar (for PhantomJS)
+
     $('<div id="modal-test"/>')
       .on('shown.bs.modal', function () {
         var currentPadding = $body.css('padding-right')
         assert.strictEqual(currentPadding, originalPadding, 'body padding should not be adjusted')
         $(this).bootstrapModal('hide')
 
-        $('#qunit-container').show()
+        // restore scrollbars
+        $body.css('overflow', 'auto')
+        $('html').css('padding-right', '16px')
         done()
       })
       .bootstrapModal('show')
@@ -544,7 +551,7 @@ $(function () {
 
     $('<div id="modal-test"/>')
       .on('hidden.bs.modal', function () {
-        assert.ok(!$body.attr('style'), 'body does not have inline padding set')
+        assert.strictEqual($body.attr('style').indexOf('padding-right'), -1, 'body does not have inline padding set')
         $style.remove()
         done()
       })