diff --git a/src/inabox/inabox-viewport.js b/src/inabox/inabox-viewport.js index c62560bf50b2e..937aa5273213a 100644 --- a/src/inabox/inabox-viewport.js +++ b/src/inabox/inabox-viewport.js @@ -359,6 +359,7 @@ export class ViewportBindingInabox { /** @override */ setScrollTop() {/* no-op */} /** @override */ getScrollWidth() {return 0;} /** @override */ getScrollHeight() {return 0;} + /** @override */ getContentHeight() {return 0;} /** @override */ getBorderTop() {return 0;} /** @override */ requiresFixedLayerTransfer() {return false;} } diff --git a/src/service/resources-impl.js b/src/service/resources-impl.js index f61225f793647..bc0d4c67b5e44 100644 --- a/src/service/resources-impl.js +++ b/src/service/resources-impl.js @@ -197,7 +197,7 @@ export class Resources { this.vsyncScheduled_ = false; /** @private {number} */ - this.scrollHeight_ = 0; + this.contentHeight_ = 0; /** @private {boolean} */ this.maybeChangeHeight_ = false; @@ -1035,10 +1035,10 @@ export class Resources { 'linkRels': Services.documentInfoForDoc(this.ampdoc).linkRels, }), /* cancelUnsent */true); - this.scrollHeight_ = this.viewport_.getScrollHeight(); + this.contentHeight_ = this.viewport_.getContentHeight(); this.viewer_.sendMessage('documentHeight', - dict({'height': this.scrollHeight_}), /* cancelUnsent */true); - dev().fine(TAG_, 'document height on load: ' + this.scrollHeight_); + dict({'height': this.contentHeight_}), /* cancelUnsent */true); + dev().fine(TAG_, 'document height on load: ' + this.contentHeight_); } const viewportSize = this.viewport_.getSize(); @@ -1065,12 +1065,12 @@ export class Resources { if (this.maybeChangeHeight_) { this.maybeChangeHeight_ = false; this.vsync_.measure(() => { - const measuredScrollHeight = this.viewport_.getScrollHeight(); - if (measuredScrollHeight != this.scrollHeight_) { + const measuredContentHeight = this.viewport_.getContentHeight(); + if (measuredContentHeight != this.contentHeight_) { this.viewer_.sendMessage('documentHeight', - dict({'height': measuredScrollHeight}), /* cancelUnsent */true); - this.scrollHeight_ = measuredScrollHeight; - dev().fine(TAG_, 'document height changed: ' + this.scrollHeight_); + dict({'height': measuredContentHeight}), /* cancelUnsent */true); + this.contentHeight_ = measuredContentHeight; + dev().fine(TAG_, 'document height changed: ' + this.contentHeight_); } }); } diff --git a/src/service/viewport/viewport-binding-def.js b/src/service/viewport/viewport-binding-def.js index 305f4f4ed10e1..150f1724f49d6 100644 --- a/src/service/viewport/viewport-binding-def.js +++ b/src/service/viewport/viewport-binding-def.js @@ -141,6 +141,15 @@ export class ViewportBindingDef { */ getScrollHeight() {} + /** + * Returns the height of the content of the document, including the + * padding top for the viewer header. + * contentHeight will match scrollHeight in all cases unless the viewport is + * taller than the content. + * @return {number} + */ + getContentHeight() {} + /** * Returns the rect of the element within the document. * @param {!Element} unusedEl diff --git a/src/service/viewport/viewport-binding-ios-embed-wrapper.js b/src/service/viewport/viewport-binding-ios-embed-wrapper.js index 205122f1c249d..76f7b4fccbd29 100644 --- a/src/service/viewport/viewport-binding-ios-embed-wrapper.js +++ b/src/service/viewport/viewport-binding-ios-embed-wrapper.js @@ -216,6 +216,15 @@ export class ViewportBindingIosEmbedWrapper_ { return this.wrapper_./*OK*/scrollHeight; } + /** @override */ + getContentHeight() { + // The reparented body inside wrapper will have the correct content height. + // Body is overflow: hidden so that the scrollHeight include the margins of + // body's first and last child. + const rect = this.win.document.body./*OK*/getBoundingClientRect(); + return rect.height + rect.top + this.getScrollTop(); + } + /** @override */ getLayoutRect(el, opt_scrollLeft, opt_scrollTop) { const scrollTop = opt_scrollTop != undefined diff --git a/src/service/viewport/viewport-binding-natural.js b/src/service/viewport/viewport-binding-natural.js index 8f82a1c20bcdd..3cd5d0c1d5af9 100644 --- a/src/service/viewport/viewport-binding-natural.js +++ b/src/service/viewport/viewport-binding-natural.js @@ -187,6 +187,15 @@ export class ViewportBindingNatural_ { return this.getScrollingElement_()./*OK*/scrollHeight; } + /** @override */ + getContentHeight() { + // The reparented body inside wrapper will have the correct content height. + // Body is overflow: hidden so that the scrollHeight include the margins of + // body's first and last child. + const rect = this.win.document.body./*OK*/getBoundingClientRect(); + return rect.height + rect.top + this.getScrollTop(); + } + /** @override */ getLayoutRect(el, opt_scrollLeft, opt_scrollTop) { const scrollTop = opt_scrollTop != undefined diff --git a/src/service/viewport/viewport-impl.js b/src/service/viewport/viewport-impl.js index b085c66bc7b0f..bbe85edaffddc 100644 --- a/src/service/viewport/viewport-impl.js +++ b/src/service/viewport/viewport-impl.js @@ -357,6 +357,19 @@ export class Viewport { return this.binding_.getScrollHeight(); } + /** + * Returns the height of the content of the document, including the + * padding top for the viewer header. + * contentHeight will match scrollHeight in all cases unless the viewport is + * taller than the content. + * Note that this method is not cached since we there's no indication when + * it might change. + * @return {number} + */ + getContentHeight() { + return this.binding_.getContentHeight(); + } + /** * Returns the rect of the viewport which includes scroll positions and size. * @return {!../../layout-rect.LayoutRectDef}} diff --git a/test/functional/test-resources.js b/test/functional/test-resources.js index f29a09f108b46..44b5a2c4f986e 100644 --- a/test/functional/test-resources.js +++ b/test/functional/test-resources.js @@ -1603,7 +1603,7 @@ describe('Resources discoverWork', () => { }); }); -describes.realWin('Resources scrollHeight', { +describes.realWin('Resources contentHeight', { amp: { runtimeOn: true, }, @@ -1621,15 +1621,15 @@ describes.realWin('Resources scrollHeight', { }); }); - it('should measure initial scrollHeight', () => { - const scrollHeight = resources.viewport_.getScrollHeight(); + it('should measure initial contentHeight', () => { + const contentHeight = resources.viewport_.getContentHeight(); expect(resources.maybeChangeHeight_).to.equal(false); expect(resources.documentReady_).to.equal(true); - expect(resources.scrollHeight_).to.equal(scrollHeight); + expect(resources.contentHeight_).to.equal(contentHeight); }); - it('should send scrollHeight to viewer if height was changed', () => { - sandbox.stub(resources.viewport_, 'getScrollHeight', () => { + it('should send contentHeight to viewer if height was changed', () => { + sandbox.stub(resources.viewport_, 'getContentHeight', () => { return 200; }); resources.maybeChangeHeight_ = true; @@ -1637,33 +1637,33 @@ describes.realWin('Resources scrollHeight', { resources.doPass(); expect(resources.maybeChangeHeight_).to.equal(false); - expect(resources.scrollHeight_).to.equal(200); + expect(resources.contentHeight_).to.equal(200); expect(viewerSendMessageStub).to.be.calledOnce; expect(viewerSendMessageStub.lastCall.args[0]).to.equal('documentHeight'); expect(viewerSendMessageStub.lastCall.args[1].height).to.equal(200); expect(viewerSendMessageStub.lastCall.args[2]).to.equal(true); }); - it('should not send scrollHeight to viewer if height is not changed', () => { - const scrollHeight = resources.viewport_.getScrollHeight(); + it('should not send contentHeight to viewer if height is not changed', () => { + const contentHeight = resources.viewport_.getContentHeight(); resources.maybeChangeHeight_ = true; resources.doPass(); expect(resources.maybeChangeHeight_).to.equal(false); - expect(resources.scrollHeight_).to.equal(scrollHeight); + expect(resources.contentHeight_).to.equal(contentHeight); expect(viewerSendMessageStub).to.not.be.called; }); - it('should send scrollHeight to viewer if viewport resizes', () => { - sandbox.stub(resources.viewport_, 'getScrollHeight', () => { + it('should send contentHeight to viewer if viewport resizes', () => { + sandbox.stub(resources.viewport_, 'getContentHeight', () => { return 200; }); resources.viewport_.changed_(/* relayoutAll */ true, /* velocity */ 0); resources.doPass(); expect(resources.maybeChangeHeight_).to.equal(false); - expect(resources.scrollHeight_).to.equal(200); + expect(resources.contentHeight_).to.equal(200); expect(viewerSendMessageStub).to.be.calledOnce; expect(viewerSendMessageStub.lastCall.args[0]).to.equal('documentHeight'); expect(viewerSendMessageStub.lastCall.args[1].height).to.equal(200); diff --git a/test/functional/test-viewport-binding.js b/test/functional/test-viewport-binding.js index 7fb9026b39f5f..9743f993cce91 100644 --- a/test/functional/test-viewport-binding.js +++ b/test/functional/test-viewport-binding.js @@ -32,6 +32,7 @@ describes.realWin('ViewportBindingNatural', {ampCss: true}, env => { let win; let ampdoc; let viewer; + let child; beforeEach(() => { env.iframe.style.width = '100px'; @@ -39,7 +40,7 @@ describes.realWin('ViewportBindingNatural', {ampCss: true}, env => { win = env.win; win.document.documentElement.classList.add('i-amphtml-singledoc'); - const child = win.document.createElement('div'); + child = win.document.createElement('div'); child.style.width = '200px'; child.style.height = '300px'; win.document.body.appendChild(child); @@ -121,6 +122,18 @@ describes.realWin('ViewportBindingNatural', {ampCss: true}, env => { expect(binding.getScrollHeight()).to.equal(300); }); + it('should calculate contentHeight from body height', () => { + // Set content to be smaller than viewport. + child.style.height = '50px'; + expect(binding.getContentHeight()).to.equal(50); + }); + + it('should include padding top in contentHeight', () => { + binding.updatePaddingTop(10); + binding.setScrollTop(20); // should have no effect on height + expect(binding.getContentHeight()).to.equal(310); + }); + it('should update scrollTop on scrollElement', () => { win.pageYOffset = 11; win.document.scrollingElement.scrollTop = 17; @@ -338,6 +351,18 @@ describes.realWin('ViewportBindingIosEmbedWrapper', {ampCss: true}, env => { expect(binding.getScrollHeight()).to.equal(301); // +1px for border-top. }); + it('should calculate contentHeight from body height', () => { + // Set content to be smaller than viewport. + child.style.height = '50px'; + expect(binding.getContentHeight()).to.equal(51); + }); + + it('should include padding top in contentHeight', () => { + binding.updatePaddingTop(10); + binding.setScrollTop(20); // should have no effect on height + expect(binding.getContentHeight()).to.equal(311); // +1px for border-top. + }); + it('should update scrollTop on wrapper', () => { binding.setScrollTop(21); expect(binding.wrapper_.scrollTop).to.equal(21); diff --git a/test/functional/test-viewport.js b/test/functional/test-viewport.js index 393b1ceb0ca59..0632259f54395 100644 --- a/test/functional/test-viewport.js +++ b/test/functional/test-viewport.js @@ -885,6 +885,12 @@ describes.fakeWin('Viewport', {}, env => { expect(viewport.getScrollHeight()).to.equal(117); }); + it('should delegate contentHeight', () => { + const bindingMock = sandbox.mock(binding); + bindingMock.expects('getContentHeight').withArgs().returns(117).once(); + expect(viewport.getContentHeight()).to.equal(117); + }); + it('should scroll to target position when the viewer sets scrollTop', () => { const bindingMock = sandbox.mock(binding); bindingMock.expects('setScrollTop').withArgs(117).once();