fix(duckplayer): ensure overlay shows when DOM is added incrementally (#593)

shakyShane · Shane Osbourne · web-flow · commit 4f851789bb46 · 2023-06-26T20:46:48.000+01:00
Co-authored-by: Shane Osbourne &lt;sosbourne@duckduckgo.com&gt;
diff --git a/.gitignore b/.gitignore
@@ -4,4 +4,6 @@ node_modules/
 .env
 build/
 docs/
+test-results/
+playwright-report/
 Sources/ContentScopeScripts/dist/
diff --git a/integration-test/playwright/duckplayer.spec.js b/integration-test/playwright/duckplayer.spec.js
@@ -108,6 +108,19 @@ test.describe('Duck Player Overlays on Video Player in YouTube.com', () => {
         // Then then the overlay shows and blocks the video from playing
         await overlays.overlayBlocksVideo()
     })
+    test('Overlay blocks video from playing (supporting DOM appearing over time)', async ({ page }, workerInfo) => {
+        const overlays = DuckplayerOverlays.create(page, workerInfo)
+
+        // Given overlays feature is enabled
+        await overlays.overlaysEnabled()
+
+        // And my setting is 'always ask'
+        await overlays.userSettingIs('always ask')
+        await overlays.gotoPlayerPage({ variant: 'incremental-dom' })
+
+        // Then then the overlay shows and blocks the video from playing
+        await overlays.overlayBlocksVideo()
+    })
     test('Overlay is removed when new settings arrive', async ({ page }, workerInfo) => {
         const overlays = DuckplayerOverlays.create(page, workerInfo)
 
diff --git a/integration-test/playwright/page-objects/duckplayer-overlays.js b/integration-test/playwright/page-objects/duckplayer-overlays.js
@@ -54,8 +54,19 @@ export class DuckplayerOverlays {
         await this.page.goto(this.overlaysPage)
     }
 
-    async gotoPlayerPage () {
-        await this.page.goto(this.playerPage + '?videoID=123')
+    /**
+     * @param {object} [params]
+     * @param {"default" | "incremental-dom"} [params.variant]
+     *  - we are replicating different strategies in the HTML to capture regressions/bugs
+     */
+    async gotoPlayerPage (params = {}) {
+        const { variant = 'default' } = params
+        const urlParams = new URLSearchParams([
+            ['videoID', '123'],
+            ['variant', variant]
+        ])
+
+        await this.page.goto(this.playerPage + '?' + urlParams.toString())
     }
 
     async gotoSerpProxyPage () {
diff --git a/integration-test/test-pages/duckplayer/pages/player.html b/integration-test/test-pages/duckplayer/pages/player.html
@@ -6,20 +6,33 @@
     <title>Duck Player - Player Overlay</title>
     <link rel="stylesheet" href="../../shared/style.css">
     <style>
+        *, *:before, *:after {
+            box-sizing: border-box;
+        }
         .container {
-            width: 800px;
-            height: 600px;
+            max-width: 800px;
+            aspect-ratio: 16/9;
             background: blue;
         }
+        #player {
+            height: 100%;
+        }
         .html5-video-player {
             position: relative;
+            height: 100%;
+            border: 2px dotted red;
         }
         body {
             max-width: 100%;
         }
         .tools {
             margin-bottom: 1rem;
         }
+        video {
+            display: block;
+            height: 100%;
+            width: 100%;
+        }
     </style>
 </head>
 <body>
@@ -39,8 +52,36 @@
         </div>
     </div>
 </template>
+
 <script type="module">
-    document.querySelector('main').innerHTML = document.querySelector('template[id="inner"]').innerHTML;
+    const variant = new URLSearchParams(window.location.search).get('variant') || 'default';
+    const main = document.querySelector('main');
+    const html = (selector) => document.querySelector(selector).innerHTML
+
+    const knownVariants = {
+        "default": () => {
+            main.innerHTML = html('template[id="inner"]');
+        },
+        "incremental-dom": () => {
+            main.innerHTML = `<div class="container"><div id="player"></div></div>`
+            setTimeout(() => {
+                const player = main.querySelector('#player');
+                player.innerHTML = `
+                <div class="html5-video-player">
+                    <video width="800px" height="600px"></video>
+                </div>
+                `
+            }, 100);
+        },
+    }
+
+    if (variant in knownVariants) {
+        console.log('executing page variant', variant)
+        knownVariants[variant]();
+    } else {
+        console.warn('variant not found', variant)
+    }
+
 </script>
 </body>
 </html>
diff --git a/src/features/duckplayer/video-overlay-manager.js b/src/features/duckplayer/video-overlay-manager.js
@@ -76,21 +76,6 @@ export class VideoOverlayManager {
         })
     }
 
-    /**
-     * Set up the overlay
-     * @param {import("../duck-player.js").UserValues} userValues
-     * @param {import("./util").VideoParams} params
-     */
-    addLargeOverlay (userValues, params) {
-        const playerVideo = document.querySelector('#player video')
-        const containerElement = document.querySelector('#player .html5-video-player')
-
-        if (playerVideo && containerElement) {
-            this.stopVideoFromPlaying(playerVideo)
-            this.appendOverlayToPage(containerElement, params)
-        }
-    }
-
     /**
      * @param {import("./util").VideoParams} params
      */
@@ -138,6 +123,15 @@ export class VideoOverlayManager {
                 return null
             }
 
+            /**
+             * Don't continue until we've been able to find the HTML elements that we inject into
+             */
+            const videoElement = playerElement.querySelector('video')
+            const playerContainer = playerElement.querySelector('.html5-video-player')
+            if (!videoElement || !playerContainer) {
+                return null
+            }
+
             /**
              * If we get here, it's a valid situation
              */
@@ -158,7 +152,8 @@ export class VideoOverlayManager {
             if ('alwaysAsk' in userValues.privatePlayerMode) {
                 if (!userValues.overlayInteracted) {
                     if (!this.environment.hasOneTimeOverride()) {
-                        this.addLargeOverlay(userValues, params)
+                        this.stopVideoFromPlaying(videoElement)
+                        this.appendOverlayToPage(playerContainer, params)
                     }
                 } else {
                     this.addSmallDaxOverlay(params)
