🐛 Avoid [overflow] layout shift after buildCallback (#27736)

* Set [overflow] to display:initial. * Add integration test. * Make sure overflow rect doesn't have 0 height. * Add comment. * Allow +/- 1.
ampproject · Apr 16, 2020 · b94427c · b94427c
1 parent 1fcc9b1
commit b94427c
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 0 deletions.
diff --git a/css/ampshared.css b/css/ampshared.css
@@ -231,6 +231,7 @@ i-amphtml-sizer {
   color: transparent !important;
 }
 
+/* Hide all children of non-container elements. */
 .i-amphtml-notbuilt:not(.i-amphtml-layout-container) > * ,
 [layout]:not([layout=container]):not(.i-amphtml-element) > *
 {
@@ -350,6 +351,10 @@ i-amphtml-scroll-container.amp-active, i-amp-scroll-container.amp-active {
   position: relative;
   z-index: 2;
   visibility: hidden;
+  /* display:initial overrides display:none from the "hide all children of
+     non-container elements" rule. This avoids a minor content jump after
+     element build since overflows increase the height of their parents. */
+  display: initial;
 }
 
 .i-amphtml-element > [overflow].amp-visible {

diff --git a/test/fixtures/overflow.html b/test/fixtures/overflow.html
@@ -0,0 +1,23 @@
+<!doctype html>
+<html ⚡>
+<head>
+  <meta charset="utf-8">
+  <title>amp-iframe</title>
+  <link rel="canonical" href="amps.html" >
+  <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
+  <script async src="/dist/amp.js"></script>
+  <script async custom-element="amp-iframe" src="intentionally_wrong"></script>
+  <style amp-custom>
+    div[overflow] {
+      height: 33px;
+    }
+  </style>
+  <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
+</head>
+<body>
+  <amp-iframe width=100 height=100 layout=responsive>
+      <div overflow>The height of the overflow should be reflected in my responsive parent's height!</div>
+      <amp-img layout="fill" src="https://lh3.googleusercontent.com/pSECrJ82R7-AqeBCOEPGPM9iG9OEIQ_QXcbubWIOdkY=w400-h300-no-n" placeholder></amp-img>
+  </amp-iframe>
+</body>
+</html>
diff --git a/test/integration/test-css.js b/test/integration/test-css.js
@@ -0,0 +1,46 @@
+/**
+ * Copyright 2020 The AMP HTML Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS-IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {AmpEvents} from '../../src/amp-events.js';
+import {createFixtureIframe} from '../../testing/iframe.js';
+
+describe
+  .configure()
+  .retryOnSaucelabs()
+  .run('CSS', () => {
+    it('should include height of [overflow] child in size before build', async () => {
+      const fixture = await createFixtureIframe(
+        'test/fixtures/overflow.html',
+        500
+      );
+      // Wait until layout.js CSS is applied.
+      await fixture.awaitEvent(AmpEvents.ATTACHED, 1);
+      const {doc} = fixture;
+
+      const iframe = doc.querySelector('amp-iframe');
+      const iframeRect = iframe.getBoundingClientRect();
+
+      const overflow = doc.querySelector('[overflow]');
+      const overflowRect = overflow.getBoundingClientRect();
+
+      expect(overflowRect.height).to.be.greaterThan(0);
+      // The amp-iframe has a 1:1 aspect ratio, and its height should be
+      // incremented by the overflow's height.
+      expect(
+        Math.abs(iframeRect.width + overflowRect.height) - iframeRect.height
+      ).to.lessThan(2);
+    });
+  });