Merge pull request #1649 from dvoytenko/noscrolling

Horizontal scrolling when embedded as an IFrame

css/amp.css

@@ -14,14 +14,21 @@
  * limitations under the License.
  */
 
+/**
+ * Horizontal scrolling interferes with embedded scenarios and predominantly
+ * the result of the non-responsive design.
+ */
+html, body {
+  overflow-x: hidden !important;
+}
+
 /**
  * Margin:0 is currently needed for iOS viewer embeds.
  * See:
  * https://github.com/ampproject/amphtml/blob/master/spec/amp-html-layout.md
  * and {@link ViewportBindingNaturalIosEmbed_} for more info.
  */
 body {
-  /* Undoes mandatory opacity: 0 in doc. */
   margin: 0 !important;
 }
 

src/service/viewport-impl.js

@@ -678,9 +678,6 @@ export class ViewportBindingNaturalIosEmbed_ {
     /** @const {!Window} */
     this.win = win;
 
-    /** @private {number} */
-    this.scrollWidth_ = 0;
-
     /** @private {?Element} */
     this.scrollPosEl_ = null;
 
@@ -713,9 +710,6 @@ export class ViewportBindingNaturalIosEmbed_ {
     const documentElement = this.win.document.documentElement;
     const documentBody = this.win.document.body;
 
-    // TODO(dvoytenko): need to also find a way to do this on resize.
-    this.scrollWidth_ = documentBody./*OK*/scrollWidth || 0;
-
     // Embedded scrolling on iOS is rather complicated. IFrames cannot be sized
     // and be scrollable. Sizing iframe by scrolling height has a big negative
     // that "fixed" position is essentially impossible. The only option we
@@ -731,11 +725,11 @@ export class ViewportBindingNaturalIosEmbed_ {
     //   -webkit-overflow-scrolling: touch;
     // }
     setStyles(documentElement, {
-      overflow: 'auto',
+      overflowY: 'auto',
       webkitOverflowScrolling: 'touch'
     });
     setStyles(documentBody, {
-      overflow: 'auto',
+      overflowY: 'auto',
       webkitOverflowScrolling: 'touch',
       position: 'absolute',
       top: 0,
@@ -837,7 +831,8 @@ export class ViewportBindingNaturalIosEmbed_ {
 
   /** @override */
   getScrollWidth() {
-    return Math.max(this.scrollWidth_, this.win./*OK*/innerWidth);
+    // There's no good way to calculate scroll width on iOS in this mode.
+    return this.win./*OK*/innerWidth;
   }
 
   /** @override */

test/functional/test-viewport.js

@@ -440,6 +440,7 @@ describe('ViewportBindingNatural', () => {
   let windowMock;
   let binding;
   let windowApi;
+  let documentElement;
   let windowEventHandlers;
 
   beforeEach(() => {
@@ -450,6 +451,12 @@ describe('ViewportBindingNatural', () => {
       windowEventHandlers[eventType] = handler;
     };
     windowApi = new WindowApi();
+    documentElement = {
+      style: {}
+    };
+    windowApi.document = {
+      documentElement: documentElement
+    };
     windowMock = sandbox.mock(windowApi);
     binding = new ViewportBindingNatural_(windowApi);
   });
@@ -620,25 +627,16 @@ describe('ViewportBindingNaturalIosEmbed', () => {
     expect(bodyEventListeners['scroll']).to.not.equal(undefined);
   });
 
-  it('should pre-calculate scrollWidth', () => {
-    expect(binding.scrollWidth_).to.equal(777);
-  });
-
-  it('should defer scrollWidth to max of window.innerHeight ' +
-        ' and body.scrollWidth', () => {
-    binding.scrollWidth_ = 0;
+  it('should always have scrollWidth equal window.innerWidth', () => {
     expect(binding.getScrollWidth()).to.equal(555);
-
-    binding.scrollWidth_ = 1000;
-    expect(binding.getScrollWidth()).to.equal(1000);
   });
 
   it('should setup document for embed scrolling', () => {
     const documentElement = windowApi.document.documentElement;
     const body = windowApi.document.body;
-    expect(documentElement.style.overflow).to.equal('auto');
+    expect(documentElement.style.overflowY).to.equal('auto');
     expect(documentElement.style.webkitOverflowScrolling).to.equal('touch');
-    expect(body.style.overflow).to.equal('auto');
+    expect(body.style.overflowY).to.equal('auto');
     expect(body.style.webkitOverflowScrolling).to.equal('touch');
     expect(body.style.position).to.equal('absolute');
     expect(body.style.top).to.equal(0);