Remove scroll-height-bounce and scroll-height-minheight experiments

ampproject · Apr 5, 2019 · 5193d64 · 5193d64
1 parent 7793c82
commit 5193d64
Show file tree

Hide file tree

Showing 5 changed files with 0 additions and 119 deletions.
diff --git a/css/amp.css b/css/amp.css
@@ -196,18 +196,6 @@ html.i-amphtml-singledoc.i-amphtml-ios-embed-sd > body {
   overflow: hidden !important;
 }
 
-/**
- * TODO(dvoytenko, #19004): if/when "scroll-height-minheight" launched, merge
- * these styles into _both_: `#i-amphtml-wrapper > body` and
- * `html.i-amphtml-ios-embed-sd > body` sections above.
- */
-.i-amphtml-body-minheight > body {
-  /* A slightly overflowing height is necessary to avoid scrolling and
-     rendering bugs on iOS due to
-     https://bugs.webkit.org/show_bug.cgi?id=158342. */
-  min-height: calc(100vh + 1px);
-}
-
 
 .i-amphtml-element {
   display: inline-block;

diff --git a/src/service/viewport/viewport-binding-ios-embed-sd.js b/src/service/viewport/viewport-binding-ios-embed-sd.js
@@ -109,9 +109,6 @@ export class ViewportBindingIosEmbedShadowRoot_ {
     const doc = this.win.document;
     const {documentElement} = doc;
     documentElement.classList.add('i-amphtml-ios-embed-sd');
-    if (isExperimentOn(win, 'scroll-height-minheight')) {
-      documentElement.classList.add('i-amphtml-body-minheight');
-    }
 
     const scroller = htmlFor(doc)`
       <div id="i-amphtml-scroller">
@@ -413,20 +410,6 @@ export class ViewportBindingIosEmbedShadowRoot_ {
 
   /** @override */
   contentHeightChanged() {
-    if (isExperimentOn(this.win, 'scroll-height-bounce')) {
-      // Refresh the overscroll (`-webkit-overflow-scrolling: touch`) to avoid
-      // iOS rendering bugs. See #8798 for details.
-      this.vsync_.mutate(() => {
-        setImportantStyles(this.scroller_, {
-          '-webkit-overflow-scrolling': 'auto',
-        });
-        this.vsync_.mutate(() => {
-          setImportantStyles(this.scroller_, {
-            '-webkit-overflow-scrolling': 'touch',
-          });
-        });
-      });
-    }
   }
 
   /** @override */

diff --git a/src/service/viewport/viewport-binding-ios-embed-wrapper.js b/src/service/viewport/viewport-binding-ios-embed-wrapper.js
@@ -57,9 +57,6 @@ export class ViewportBindingIosEmbedWrapper_ {
     this.wrapper_ = wrapper;
     wrapper.id = 'i-amphtml-wrapper';
     wrapper.className = topClasses;
-    if (isExperimentOn(win, 'scroll-height-minheight')) {
-      wrapper.classList.add('i-amphtml-body-minheight');
-    }
 
     /** @private @const {!Observable} */
     this.scrollObservable_ = new Observable();
@@ -255,18 +252,6 @@ export class ViewportBindingIosEmbedWrapper_ {
 
   /** @override */
   contentHeightChanged() {
-    if (isExperimentOn(this.win, 'scroll-height-bounce')) {
-      // Refresh the overscroll (`-webkit-overflow-scrolling: touch`) to avoid
-      // iOS rendering bugs. See #8798 for details.
-      const doc = this.win.document;
-      const {documentElement} = doc;
-      this.vsync_.mutate(() => {
-        documentElement.classList.remove('i-amphtml-ios-overscroll');
-        this.vsync_.mutate(() => {
-          documentElement.classList.add('i-amphtml-ios-overscroll');
-        });
-      });
-    }
   }
 
   /** @override */

diff --git a/test/unit/test-viewport-binding.js b/test/unit/test-viewport-binding.js
@@ -233,7 +233,6 @@ describes.realWin('ViewportBindingIosEmbedWrapper', {ampCss: true}, env => {
     env.iframe.style.height = '100px';
     win = env.win;
     win.document.documentElement.className = 'top i-amphtml-singledoc';
-    toggleExperiment(win, 'scroll-height-minheight', false);
     child = win.document.createElement('div');
     child.style.width = '200px';
     child.style.height = '300px';
@@ -252,17 +251,6 @@ describes.realWin('ViewportBindingIosEmbedWrapper', {ampCss: true}, env => {
     expect(style.minHeight).to.equal('0px');
   });
 
-  it('should setup body min-height wwith experiment', () => {
-    toggleExperiment(win, 'scroll-height-minheight', true);
-    try {
-      binding = new ViewportBindingIosEmbedWrapper_(win);
-    } catch (e) {
-      // Ignore a double-init errors.
-    }
-    const style = win.getComputedStyle(win.document.body);
-    expect(style.minHeight).to.equal((win.innerHeight + 1) + 'px');
-  });
-
   it('should NOT require fixed layer transferring', () => {
     expect(binding.requiresFixedLayerTransfer()).to.be.true;
   });
@@ -477,24 +465,7 @@ describes.realWin('ViewportBindingIosEmbedWrapper', {ampCss: true}, env => {
     expect(wrapperCss.overflowY).to.equal('auto');
   });
 
-  it('should refresh overscroll when content height changes', () => {
-    toggleExperiment(win, 'scroll-height-bounce', true);
-    const root = win.document.documentElement;
-    return vsync.mutatePromise().then(() => {
-      expect(root).to.have.class('i-amphtml-ios-overscroll');
-      binding.contentHeightChanged();
-      return vsync.mutatePromise();
-    }).then(() => {
-      expect(root).to.not.have.class('i-amphtml-ios-overscroll');
-      return vsync.mutatePromise();
-    }).then(() => {
-      expect(root).to.have.class('i-amphtml-ios-overscroll');
-    });
-  });
-
   it('should NOT refresh overscroll w/o experiment', () => {
-    // TODO(#19004): cleanup once "scroll-height-bounce" is launched.
-    toggleExperiment(win, 'scroll-height-bounce', false);
     binding.contentHeightChanged();
     const root = win.document.documentElement;
     return vsync.mutatePromise().then(() => {
@@ -518,7 +489,6 @@ describes.realWin('ViewportBindingIosEmbedShadowRoot_', {ampCss: true}, env => {
       iframe.style.width = '100px';
       iframe.style.height = '100px';
       win = env.win;
-      toggleExperiment(win, 'scroll-height-minheight', false);
       win.document.documentElement.className = 'top i-amphtml-singledoc';
       child = win.document.createElement('div');
       child.style.width = '200px';
@@ -548,17 +518,6 @@ describes.realWin('ViewportBindingIosEmbedShadowRoot_', {ampCss: true}, env => {
       expect(style.minHeight).to.equal('0px');
     });
 
-    it('should setup body min-height wwith experiment', () => {
-      toggleExperiment(win, 'scroll-height-minheight', true);
-      try {
-        new ViewportBindingIosEmbedShadowRoot_(win);
-      } catch (e) {
-        // Ignore a double-init errors.
-      }
-      const style = win.getComputedStyle(win.document.body);
-      expect(style.minHeight).to.equal((win.innerHeight + 1) + 'px');
-    });
-
     it('should NOT require fixed layer transferring', () => {
       expect(binding.requiresFixedLayerTransfer()).to.be.true;
     });
@@ -825,28 +784,7 @@ describes.realWin('ViewportBindingIosEmbedShadowRoot_', {ampCss: true}, env => {
       expect(scrollerCss.overflowY).to.equal('auto');
     });
 
-    it('should refresh overscroll when content height changes', () => {
-      toggleExperiment(win, 'scroll-height-bounce', true);
-      const scroller = binding.scroller_;
-      const setStyleStub = sandbox.stub(scroller.style, 'setProperty');
-      return vsync.mutatePromise().then(() => {
-        binding.contentHeightChanged();
-        return vsync.mutatePromise();
-      }).then(() => {
-        const {args} = setStyleStub.lastCall;
-        expect(args[0]).to.equal('-webkit-overflow-scrolling');
-        expect(args[1]).to.equal('auto');
-        return vsync.mutatePromise();
-      }).then(() => {
-        const {args} = setStyleStub.lastCall;
-        expect(args[0]).to.equal('-webkit-overflow-scrolling');
-        expect(args[1]).to.equal('touch');
-      });
-    });
-
     it('should NOT refresh overscroll w/o experiment', () => {
-      // TODO(#19004): cleanup once "scroll-height-bounce" is launched.
-      toggleExperiment(win, 'scroll-height-bounce', false);
       const scroller = binding.scroller_;
       const setStyleStub = sandbox.stub(scroller.style, 'setProperty');
       binding.contentHeightChanged();

diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js
@@ -393,19 +393,6 @@ const EXPERIMENTS = [
     spec: 'https://github.com/ampproject/amphtml/issues/17475',
     cleanupIssue: 'https://github.com/ampproject/amphtml/issues/18897',
   },
-  {
-    id: 'scroll-height-bounce',
-    name: 'Bounces the scrolling when scroll height changes' +
-        ' (fix for #18861 and #8798)',
-    spec: 'https://github.com/ampproject/amphtml/issues/18861',
-    cleanupIssue: 'https://github.com/ampproject/amphtml/issues/19004',
-  },
-  {
-    id: 'scroll-height-minheight',
-    name: 'Forces min-height on body (fix for #18861 and #8798)',
-    spec: 'https://github.com/ampproject/amphtml/issues/18861',
-    cleanupIssue: 'https://github.com/ampproject/amphtml/issues/19004',
-  },
   {
     id: 'amp-auto-lightbox',
     name: 'Automatically detects images to place in a lightbox.',