Revert "🐛 Report zero visibility for zero size element (#30105)" (#30284

) This reverts commit 584158c.
ampproject · Sep 17, 2020 · 18ec044 · 18ec044
1 parent d83351b
commit 18ec044
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 76 deletions.
diff --git a/extensions/amp-analytics/0.1/test/test-amp-analytics.js b/extensions/amp-analytics/0.1/test/test-amp-analytics.js
@@ -1468,11 +1468,8 @@ describes.realWin(
     describe('Sandbox AMP Analytics Element', () => {
       beforeEach(() => {
         // Unfortunately need to fake sandbox analytics element's parent
-        // to an AMP element.
-        // Set the doc width/height to 1 to trigger visible event.
+        // to an AMP element
         doc.body.classList.add('i-amphtml-element');
-        doc.body.style.minWidth = '1px';
-        doc.body.style.minHeight = '1px';
       });
 
       afterEach(() => {
@@ -1754,10 +1751,6 @@ describes.realWin(
     describe('parentPostMessage', () => {
       let postMessageSpy;
 
-      beforeEach(() => {
-        doc.body.style.minWidth = '1px';
-        doc.body.style.minHeight = '1px';
-      });
       function waitForParentPostMessage(opt_max) {
         if (postMessageSpy.callCount) {
           return Promise.resolve();

diff --git a/extensions/amp-analytics/0.1/test/test-visibility-manager.js b/extensions/amp-analytics/0.1/test/test-visibility-manager.js
@@ -202,7 +202,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target: otherTarget,
         intersectionRatio: 0.3,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(root.getRootVisibility()).to.equal(0);
@@ -213,13 +212,11 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target: otherTarget,
         intersectionRatio: 0.5,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
       {
         target: win.document.documentElement,
         intersectionRatio: 0.3,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(root.getRootVisibility()).to.equal(0.3);
@@ -230,7 +227,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target: win.document.documentElement,
         intersectionRatio: 0,
         intersectionRect: layoutRectLtwh(0, 0, 0, 0),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(root.getRootVisibility()).to.equal(0);
@@ -529,7 +525,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target,
         intersectionRatio: 0.3,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(model.getVisibility_()).to.equal(0.3);
@@ -574,7 +569,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target,
         intersectionRatio: 0.3,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(model.getVisibility_()).to.equal(0.3);
@@ -585,7 +579,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target,
         intersectionRatio: -0.01,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(model.getVisibility_()).to.equal(0);
@@ -595,7 +588,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target,
         intersectionRatio: -1000,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(model.getVisibility_()).to.equal(0);
@@ -606,7 +598,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target,
         intersectionRatio: 1.01,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(model.getVisibility_()).to.equal(1);
@@ -616,55 +607,11 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target,
         intersectionRatio: 1000,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(model.getVisibility_()).to.equal(1);
   });
 
-  it('should protect from zero size element', () => {
-    const target = win.document.createElement('div');
-    root.listenElement(target, {}, null, null, eventResolver);
-    expect(root.models_).to.have.length(1);
-    const model = root.models_[0];
-
-    const inOb = root.getIntersectionObserver_();
-    expect(model.getVisibility_()).to.equal(0);
-
-    // Valid element size
-    inOb.callback([
-      {
-        target,
-        intersectionRatio: 1,
-        intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 10, 10),
-      },
-    ]);
-    expect(model.getVisibility_()).to.equal(1);
-
-    // zero element size.
-    inOb.callback([
-      {
-        target,
-        intersectionRatio: 1,
-        intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 0, 10),
-      },
-    ]);
-    expect(model.getVisibility_()).to.equal(0);
-
-    // zero element size, high resolution screen.
-    inOb.callback([
-      {
-        target,
-        intersectionRatio: 0.8,
-        intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 10, 0.452),
-      },
-    ]);
-    expect(model.getVisibility_()).to.equal(0);
-  });
-
   it('should listen on a element with different specs', () => {
     clock.tick(1);
     const inOb = root.getIntersectionObserver_();
@@ -690,7 +637,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target,
         intersectionRatio: 0.3,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
     expect(model1.getVisibility_()).to.equal(0.3);
@@ -766,7 +712,6 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, (env) => {
         target,
         intersectionRatio: 0.3,
         intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-        boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
       },
     ]);
 
@@ -950,7 +895,6 @@ describes.realWin(
           target: embed.host,
           intersectionRatio: 0.5,
           intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-          boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
         },
       ]);
       expect(root.getRootVisibility()).to.equal(0.5);
@@ -963,7 +907,6 @@ describes.realWin(
           target: otherTarget,
           intersectionRatio: 0.45,
           intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-          boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
         },
       ]);
       expect(root.getRootVisibility()).to.equal(0.5);
@@ -990,7 +933,6 @@ describes.realWin(
           target: embed.host,
           intersectionRatio: 0,
           intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-          boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
         },
       ]);
       expect(root.getRootVisibility()).to.equal(0);
@@ -1003,7 +945,6 @@ describes.realWin(
           target: otherTarget,
           intersectionRatio: 0.55,
           intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-          boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
         },
       ]);
       expect(root.getRootVisibility()).to.equal(0);
@@ -1016,7 +957,6 @@ describes.realWin(
           target: embed.host,
           intersectionRatio: 0.7,
           intersectionRect: layoutRectLtwh(0, 0, 1, 1),
-          boundingClientRect: layoutRectLtwh(1, 1, 1, 1),
         },
       ]);
       expect(root.getRootVisibility()).to.equal(0.7);

diff --git a/extensions/amp-analytics/0.1/visibility-manager.js b/extensions/amp-analytics/0.1/visibility-manager.js
@@ -872,6 +872,7 @@ export class VisibilityManagerForDoc extends VisibilityManager {
         Number(intersection.width),
         Number(intersection.height)
       );
+      // TODO(#29618): Remove after ampim investigation
       let {boundingClientRect} = change;
       boundingClientRect =
         boundingClientRect &&
@@ -891,6 +892,7 @@ export class VisibilityManagerForDoc extends VisibilityManager {
   }
 
   /**
+   * TODO(#29618): Clean up boundingClientRect after ampim investigation
    * @param {!Element} target
    * @param {number} intersectionRatio
    * @param {!../../../src/layout-rect.LayoutRectDef} intersectionRect
@@ -903,16 +905,9 @@ export class VisibilityManagerForDoc extends VisibilityManager {
     intersectionRect,
     boundingClientRect
   ) {
+    intersectionRatio = Math.min(Math.max(intersectionRatio, 0), 1);
     const id = getElementId(target);
     const trackedElement = this.trackedElements_[id];
-    if (boundingClientRect.width < 1 || boundingClientRect.height < 1) {
-      // Set intersectionRatio to 0 when the element is not visible.
-      // Use < 1 because the width/height can
-      // be a double value on high resolution screen
-      intersectionRatio = 0;
-    } else {
-      intersectionRatio = Math.min(Math.max(intersectionRatio, 0), 1);
-    }
     if (trackedElement) {
       trackedElement.intersectionRatio = intersectionRatio;
       trackedElement.intersectionRect = intersectionRect;