fix(util): body overflow-x breaking disableScrollAround (#9864)

crisbeto · jelbourn · commit 44681265c574 · 2016-10-21T23:21:13.000+02:00
Currently the `disableScrollAround` function measures the body in order to determine whether to hide it's overflow. This measurement can get thrown off by the `html` node's scrollbar that gets added a few lines above. This change switches to adding the scrollbar to the `html` node after the body measurements are done. Fixes #9860.
diff --git a/src/core/util/util.js b/src/core/util/util.js
@@ -283,22 +283,27 @@ function UtilFactory($document, $timeout, $compile, $rootScope, $$mdAnimate, $in
 
         var viewportTop = $mdUtil.getViewportTop();
         var clientWidth = body.clientWidth;
+        var hasVerticalScrollbar = body.scrollHeight > body.clientHeight + 1;
 
-        if (body.scrollHeight > body.clientHeight + 1) {
-
+        if (hasVerticalScrollbar) {
           angular.element(body).css({
             position: 'fixed',
             width: '100%',
             top: -viewportTop + 'px'
           });
-
-          documentElement.style.overflowY = 'scroll';
         }
 
         if (body.clientWidth < clientWidth) {
           body.style.overflow = 'hidden';
         }
 
+        // This should be applied after the manipulation to the body, because
+        // adding a scrollbar can potentially resize it, causing the measurement
+        // to change.
+        if (hasVerticalScrollbar) {
+          documentElement.style.overflowY = 'scroll';
+        }
+
         return function restoreScroll() {
           // Reset the inline style CSS to the previous.
           body.style.cssText = prevBodyStyle;
diff --git a/src/core/util/util.spec.js b/src/core/util/util.spec.js
@@ -98,7 +98,7 @@ describe('util', function() {
         };
 
         expect(function() {
-          $mdUtil.getModelOption(ngModelCtrl, 'Unknown')
+          $mdUtil.getModelOption(ngModelCtrl, 'Unknown');
         }).not.toThrow();
       }));
 
@@ -268,7 +268,7 @@ describe('util', function() {
         element.remove();
       }));
 
-      it('should not remove the element when being use as scorll mask', inject(function($mdUtil) {
+      it('should not remove the element when being use as scroll mask', inject(function($mdUtil) {
         var element = angular.element('<div>');
 
         document.body.appendChild(element[0]);
@@ -285,6 +285,27 @@ describe('util', function() {
         element.remove();
       }));
 
+      it('should not get thrown off by the scrollbar on the <html> node',
+        inject(function($mdUtil) {
+          var element = angular.element('<div style="height: 2000px">');
+
+          document.body.appendChild(element[0]);
+          document.body.style.overflowX = 'hidden';
+
+          window.scrollTo(0, 1000);
+
+          var enableScrolling = $mdUtil.disableScrollAround(element);
+
+          expect(document.body.style.overflow).not.toBe('hidden');
+
+          // Restore the scrolling.
+          enableScrolling();
+          window.scrollTo(0, 0);
+          document.body.style.overflowX = '';
+
+          element.remove();
+        })
+      );
     });
 
     describe('getViewportTop', function() {
