🐛 Correctly detect P tags as text element types (#36268)

* Correctly detect P tags as text element types * Add test case * Switch to nodeName
ampproject · Oct 6, 2021 · e51d33d · e51d33d
1 parent 5e83d4f
commit e51d33d
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 11 deletions.
diff --git a/src/service/performance-impl.js b/src/service/performance-impl.js
@@ -60,25 +60,26 @@ function getElementType(node) {
   if (node == null) {
     return ELEMENT_TYPE.other;
   }
-  const {tagName} = getOutermostAmpElement(node);
-  if (tagName == null) {
-    return ELEMENT_TYPE.text;
-  }
-  if (tagName === 'IMG' || tagName === 'AMP-IMG') {
+  const outer = getOutermostAmpElement(node);
+  const {nodeName} = outer;
+  if (nodeName === 'IMG' || nodeName === 'AMP-IMG') {
     return ELEMENT_TYPE.image;
   }
-  if (tagName === 'VIDEO' || tagName === 'AMP-VIDEO') {
+  if (nodeName === 'VIDEO' || nodeName === 'AMP-VIDEO') {
     return ELEMENT_TYPE.video;
   }
-  if (tagName === 'AMP-CAROUSEL') {
+  if (nodeName === 'AMP-CAROUSEL') {
     return ELEMENT_TYPE.carousel;
   }
-  if (tagName === 'AMP-BASE-CAROUSEL') {
+  if (nodeName === 'AMP-BASE-CAROUSEL') {
     return ELEMENT_TYPE.bcarousel;
   }
-  if (tagName === 'AMP-AD') {
+  if (nodeName === 'AMP-AD') {
     return ELEMENT_TYPE.ad;
   }
+  if (!nodeName.startsWith('AMP-') && outer.textContent) {
+    return ELEMENT_TYPE.text;
+  }
   return ELEMENT_TYPE.other;
 }
 
@@ -923,8 +924,8 @@ export class Performance {
  * Traverse node ancestors and return the highest level amp element.
  * Returns the given node if none are found.
  *
- * @param {!HTMLElement} node
- * @return {!HTMLElement}
+ * @param {!Node} node
+ * @return {!Node}
  */
 function getOutermostAmpElement(node) {
   let max = node;

diff --git a/test/unit/test-performance.js b/test/unit/test-performance.js
@@ -1095,6 +1095,23 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => {
       // Flush LCP again.
       toggleVisibility(perf, false);
 
+      // A textual paragraph
+      const p = document.createElement('p');
+      p.textContent = 'hello';
+      performanceObserver.triggerCallback({
+        getEntries() {
+          return [
+            {
+              entryType: 'largest-contentful-paint',
+              startTime: 25,
+              element: p,
+            },
+          ];
+        },
+      });
+      // Flush LCP again.
+      toggleVisibility(perf, false);
+
       const lcptEvents = perf.events_.filter(({label}) =>
         label.startsWith('lcpt')
       );
@@ -1106,6 +1123,10 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => {
         label: 'lcpt',
         delta: ELEMENT_TYPE.carousel,
       });
+      expect(lcptEvents).deep.include({
+        label: 'lcpt',
+        delta: ELEMENT_TYPE.text,
+      });
     });
   });