From d328abc7e23a8253812c9354cc7382d2a11555d1 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Wed, 25 Mar 2020 17:52:07 -0700 Subject: [PATCH] Revert "Disable x-scrolling on iOS when iframed (#27319)" (#27422) This reverts commit 17636f5a70147c20cc8b4d75d824bd508449aef5. --- css/ampdoc.css | 11 ----------- extensions/amp-iframe/0.1/test/test-amp-iframe.js | 5 ----- .../0.1/test/integration/test-amp-sidebar.js | 5 ----- .../0.2/test/integration/test-amp-sidebar.js | 5 ----- src/service/viewport/viewport-binding-natural.js | 1 - test/unit/test-viewport-binding.js | 10 ---------- 6 files changed, 37 deletions(-) diff --git a/css/ampdoc.css b/css/ampdoc.css index 7591be264a07..1eea89a61a7d 100644 --- a/css/ampdoc.css +++ b/css/ampdoc.css @@ -76,17 +76,6 @@ html.i-amphtml-fie > body { overflow: visible !important; } -/** - * iOS jitters when body overflows. See b/149328043. Notice that the min-height - * essentially cancels `height: auto` from above. - */ -html.i-amphtml-singledoc.i-amphtml-iframed:not(.i-amphtml-ios-embed):not([amp4ads]) > body { - overflow-x: hidden !important; - overflow-y: visible !important; - min-height: calc(100vh - var(--i-amphtml-padding-top, 0px)); -} - - /** * The `position: relative` is necessary to ensure that `paddingTop` can be * applied to a document to offset the header height, and it doesn't missplace diff --git a/extensions/amp-iframe/0.1/test/test-amp-iframe.js b/extensions/amp-iframe/0.1/test/test-amp-iframe.js index ed90ce376b43..fbadde1be8c5 100644 --- a/extensions/amp-iframe/0.1/test/test-amp-iframe.js +++ b/extensions/amp-iframe/0.1/test/test-amp-iframe.js @@ -77,11 +77,6 @@ describes.realWin( content = message.data.replace('content-iframe:', ''); } }); - // TODO(dvoytenko): checked manually and it works, but for some - // reason the scrolling does not trigger on the scrollElement in - // the integration test. Thus disabling `i-amphtml-iframed` here - // for now. - doc.documentElement.classList.remove('i-amphtml-iframed'); setTrackingIframeTimeoutForTesting(20); }); diff --git a/extensions/amp-sidebar/0.1/test/integration/test-amp-sidebar.js b/extensions/amp-sidebar/0.1/test/integration/test-amp-sidebar.js index 126e54312da3..681c528e3c4d 100644 --- a/extensions/amp-sidebar/0.1/test/integration/test-amp-sidebar.js +++ b/extensions/amp-sidebar/0.1/test/integration/test-amp-sidebar.js @@ -91,11 +91,6 @@ describe const openerButton = win.document.getElementById('sidebarOpener'); const sidebar = win.document.getElementById('sidebar1'); const viewport = sidebar.implementation_.getViewport(); - // TODO(dvoytenko): checked manually and it works, but for some - // reason the scrolling does not trigger on the scrollElement in - // the integration test. Thus disabling `i-amphtml-iframed` here - // for now. - win.document.documentElement.classList.remove('i-amphtml-iframed'); const openedPromise = waitForSidebarOpen(win.document); openerButton.click(); expect(viewport.getScrollTop()).to.equal(0); diff --git a/extensions/amp-sidebar/0.2/test/integration/test-amp-sidebar.js b/extensions/amp-sidebar/0.2/test/integration/test-amp-sidebar.js index 126e54312da3..681c528e3c4d 100644 --- a/extensions/amp-sidebar/0.2/test/integration/test-amp-sidebar.js +++ b/extensions/amp-sidebar/0.2/test/integration/test-amp-sidebar.js @@ -91,11 +91,6 @@ describe const openerButton = win.document.getElementById('sidebarOpener'); const sidebar = win.document.getElementById('sidebar1'); const viewport = sidebar.implementation_.getViewport(); - // TODO(dvoytenko): checked manually and it works, but for some - // reason the scrolling does not trigger on the scrollElement in - // the integration test. Thus disabling `i-amphtml-iframed` here - // for now. - win.document.documentElement.classList.remove('i-amphtml-iframed'); const openedPromise = waitForSidebarOpen(win.document); openerButton.click(); expect(viewport.getScrollTop()).to.equal(0); diff --git a/src/service/viewport/viewport-binding-natural.js b/src/service/viewport/viewport-binding-natural.js index 4ab472104aaf..52f249a9a737 100644 --- a/src/service/viewport/viewport-binding-natural.js +++ b/src/service/viewport/viewport-binding-natural.js @@ -138,7 +138,6 @@ export class ViewportBindingNatural_ { updatePaddingTop(paddingTop) { setImportantStyles(this.win.document.documentElement, { 'padding-top': px(paddingTop), - '--i-amphtml-padding-top': px(paddingTop), }); } diff --git a/test/unit/test-viewport-binding.js b/test/unit/test-viewport-binding.js index c8f43b381377..e939069071e5 100644 --- a/test/unit/test-viewport-binding.js +++ b/test/unit/test-viewport-binding.js @@ -73,16 +73,6 @@ describes.realWin('ViewportBindingNatural', {ampCss: true}, env => { expect(bodyStyles.overflowY).to.not.equal('hidden'); }); - it('should override body overflow for iframes', () => { - win.document.documentElement.classList.add('i-amphtml-iframed'); - win.document.documentElement.classList.add('i-amphtml-singledoc'); - binding = new ViewportBindingNatural_(ampdoc); - const bodyStyles = win.getComputedStyle(win.document.body); - expect(bodyStyles.position).to.equal('relative'); - expect(bodyStyles.overflowX).to.equal('hidden'); - expect(bodyStyles.overflowY).to.not.equal('hidden'); - }); - it('should NOT require fixed layer transferring', () => { expect(binding.requiresFixedLayerTransfer()).to.be.false; });