Revert part of #19947 and #19680 which caused race condition for amp-…

…analytics layout (#19963) * put amp-analytics > 3 viewports away in integration test * save a line * increase timeout, try to fix test * Revert part of #19947 and #19680
ampproject · Dec 22, 2018 · e01e846 · e01e846
1 parent 6567a67
commit e01e846
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 61 deletions.
diff --git a/bundles.config.js b/bundles.config.js
@@ -58,12 +58,7 @@ exports.extensionBundles = [
   {name: 'amp-ad-network-gmossp-impl', version: '0.1', type: TYPES.AD},
   {name: 'amp-ad-exit', version: '0.1', type: TYPES.AD},
   {name: 'amp-addthis', version: '0.1', type: TYPES.MISC},
-  {
-    name: 'amp-analytics',
-    version: '0.1',
-    options: {hasCss: true},
-    type: TYPES.MISC,
-  },
+  {name: 'amp-analytics', version: '0.1', type: TYPES.MISC},
   {name: 'amp-anim', version: '0.1', type: TYPES.MEDIA},
   {name: 'amp-animation', version: '0.1', type: TYPES.MISC},
   {

diff --git a/css/amp.css b/css/amp.css
@@ -662,9 +662,21 @@ amp-instagram {
 /**
  * Analytics & amp-story-auto-ads tags should never be visible. keep them hidden.
  */
-amp-analytics:not(.i-amphtml-element),
-amp-story-auto-ads:not(.i-amphtml-element){
-  display: none !important;
+amp-analytics, amp-story-auto-ads {
+  /* Fixed to make position independent of page other elements. */
+  position: fixed !important;
+  top: 0 !important;
+  width: 1px !important;
+  height: 1px !important;
+  overflow: hidden !important;
+  visibility: hidden;
+}
+
+html.i-amphtml-fie > amp-analytics {
+  /* Remove position fixed if it's in iframe.
+  * So runtime can measure layoutBox correctly
+  */
+  position: initial !important;
 }
 
 /**

diff --git a/extensions/amp-analytics/0.1/amp-analytics.css b/extensions/amp-analytics/0.1/amp-analytics.css
diff --git a/extensions/amp-analytics/0.1/amp-analytics.js b/extensions/amp-analytics/0.1/amp-analytics.js
@@ -17,7 +17,6 @@
 import {Activity} from './activity-impl';
 import {AnalyticsConfig, mergeObjects} from './config';
 import {AnalyticsEventType} from './events';
-import {CSS} from '../../../build/amp-analytics-0.1.css';
 import {CookieWriter} from './cookie-writer';
 import {
   ExpansionOptions,
@@ -709,5 +708,5 @@ AMP.extension(TAG, '0.1', AMP => {
   installVariableService(AMP.win);
   installLinkerReaderService(AMP.win);
   // Register the element.
-  AMP.registerElement(TAG, AmpAnalytics, CSS);
+  AMP.registerElement(TAG, AmpAnalytics);
 });
diff --git a/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.css b/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.css
@@ -20,20 +20,6 @@
   transform: scale(1.0) translateX(-100%) translateY(200%) !important;
 }
 
-/**
- * amp-story-auto-ads tags should never be visible. keep them hidden.
- */
-amp-story-auto-ads {
-  /* Fixed to make position independent of page other elements. */
-  position: fixed !important;
-  top: 0 !important;
-  left: 0 !important;
-  width: 1px !important;
-  height: 1px !important;
-  overflow: hidden !important;
-  visibility: hidden;
-}
-
 .i-amphtml-story-ad-link {
   background-color: #FFFFFF !important;
   border-radius: 20px !important;

diff --git a/test/integration/test-amp-analytics.js b/test/integration/test-amp-analytics.js
@@ -26,6 +26,8 @@ describe.configure().run('amp-analytics', function() {
         // initialize _cid cookie with a CLIENT_ID
         document.cookie='_cid=amp-12345';
       </script>
+      <!-- put amp-analytics > 3 viewports away from viewport -->
+      <div style="height: 400vh"></div>
       <amp-analytics>
         <script type="application/json">
         {