fix: issue with 'other' targets not fully loading (#28229)

ryanthemanuel · web-flow · commit 8c4a1067cb86 · 2023-11-03T11:28:49.000-05:00
diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md
@@ -6,6 +6,7 @@ _Released 11/7/2023 (PENDING)_
 **Bugfixes:**
 
 - Fixed an issue determining visibility when an element is hidden by an ancestor with a shared edge. Fixes [#27514](https://github.com/cypress-io/cypress/issues/27514).
+- Fixed an issue with 'other' targets (e.g. pdf documents embedded in an object tag) not fully loading. Fixes [#28228](https://github.com/cypress-io/cypress/issues/28228)
 - Fixed an issue where network requests made from tabs/windows other than the main Cypress tab would be delayed. Fixes [#28113](https://github.com/cypress-io/cypress/issues/28113).
 - Stopped processing CDP events at the end of a spec when Test Isolation is off and Test Replay is enabled. Addressed in [#28213](https://github.com/cypress-io/cypress/pull/28213).
 
diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts
@@ -277,7 +277,8 @@ export class BrowserCriClient {
     try {
       // The basic approach here is we attach to targets and enable network traffic
       // We must attach in a paused state so that we can enable network traffic before the target starts running.
-      if (targetInfo.type !== 'page') {
+      // We don't track child tabs/page network traffic. 'other' targets can't have network enabled
+      if (event.targetInfo.type !== 'page' && event.targetInfo.type !== 'other') {
         await browserClient.send('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS, event.sessionId)
       }
     } catch (error) {
diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts
@@ -287,7 +287,8 @@ export const create = async ({
         cri.on('Target.attachedToTarget', async (event) => {
           try {
             // Service workers get attached at the page and browser level. We only want to handle them at the browser level
-            if (event.targetInfo.type !== 'service_worker' && event.targetInfo.type !== 'page') {
+            // We don't track child tabs/page network traffic. 'other' targets can't have network enabled
+            if (event.targetInfo.type !== 'service_worker' && event.targetInfo.type !== 'page' && event.targetInfo.type !== 'other') {
               await cri.send('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS, event.sessionId)
             }
 
diff --git a/system-tests/projects/e2e/cypress/e2e/other_target.cy.js b/system-tests/projects/e2e/cypress/e2e/other_target.cy.js
@@ -0,0 +1,7 @@
+// The goal of this test is to load a page containing a target type of 'other' (e.g. embedded pdfs)
+// This is to ensure that we don't hang on the cy.visit (https://github.com/cypress-io/cypress/issues/28228)
+context('Test', () => {
+  it('visit site with unsupported content', () => {
+    cy.visit('http://localhost:1515/other_target.html')
+  })
+})
diff --git a/system-tests/projects/e2e/other-target.pdf b/system-tests/projects/e2e/other-target.pdf
diff --git a/system-tests/projects/e2e/other-target.webm b/system-tests/projects/e2e/other-target.webm
diff --git a/system-tests/projects/e2e/other_target.html b/system-tests/projects/e2e/other_target.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>mask-unsupported-content</title>
+  </head>
+  <body>
+    <!-- Video Player -->
+    <h2>Video Player</h2>
+
+    <object
+      type="video/mp4"
+      data="https://localhost:1516/other-target.webm"
+      width="320"
+      height="180">
+    </object>
+
+    <!-- Objects -->
+    <h2>Objects</h2>
+    <embed src="http://localhost:1515/other-target.pdf" width="500" height="375" type="application/pdf">
+    <object type="text/html" data="http://localhost:1515/other-target.pdf" width="250" height="200"></object>
+  </body>
+</html>
diff --git a/system-tests/test/other_target_spec.js b/system-tests/test/other_target_spec.js
@@ -0,0 +1,32 @@
+const express = require('express')
+const Fixtures = require('../lib/fixtures')
+const systemTests = require('../lib/system-tests').default
+
+const e2ePath = Fixtures.projectPath('e2e')
+
+const onServer = function (app) {
+  app.use(express.static(e2ePath, {
+    // force caching to happen
+    maxAge: 3600000,
+  }))
+}
+
+describe('e2e other target', () => {
+  systemTests.setup({
+    servers: [{
+      port: 1515,
+      onServer,
+    }, {
+      https: true,
+      port: 1516,
+      onServer,
+    }],
+  })
+
+  // The goal of this test is to load a page containing a target type of 'other' (e.g. embedded pdfs)
+  // This is to ensure that we don't hang on the cy.visit (https://github.com/cypress-io/cypress/issues/28228)
+  systemTests.it(`executes a page containing a target type of 'other'`, {
+    project: 'e2e',
+    spec: 'other_target.cy.js',
+  })
+})

Original file line number	Diff line number	Diff line change
`@@ -287,7 +287,8 @@ export const create = async ({`
`287`	`287`	`cri.on('Target.attachedToTarget', async (event) => {`
`288`	`288`	`try {`
`289`	`289`	`// Service workers get attached at the page and browser level. We only want to handle them at the browser level`
`290`		`- if (event.targetInfo.type !== 'service_worker' && event.targetInfo.type !== 'page') {`
	`290`	`+ // We don't track child tabs/page network traffic. 'other' targets can't have network enabled`
	`291`	`+ if (event.targetInfo.type !== 'service_worker' && event.targetInfo.type !== 'page' && event.targetInfo.type !== 'other') {`
`291`	`292`	`await cri.send('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS, event.sessionId)`
`292`	`293`	`}`
`293`	`294`