fix(duckplayer): fix clicks on shorts (#619)

Co-authored-by: Shane Osbourne <[email protected]>

Author: shakyShane

integration-test/playwright/duckplayer.e2e.spec.js

@@ -19,4 +19,24 @@ test.describe('e2e: Duck Player Thumbnail Overlays on YouTube.com', () => {
         await overlays.hoverShort()
         await overlays.overlaysDontShow()
     })
+    test('control (without our script): clicking on a short loads correctly', async ({ page }, workerInfo) => {
+        // @ts-expect-error - TS doesn't know about the "use.e2e" property
+        workerInfo.skip(!workerInfo.project.use?.e2e)
+        const overlays = DuckplayerOverlays.create(page, workerInfo)
+        await overlays.gotoYoutubeHomepage()
+        await page.waitForTimeout(2000)
+        await overlays.clicksFirstShortsThumbnail()
+        await overlays.showsShortsPage()
+    })
+    test('e2e: when enabled, clicking shorts has no impact', async ({ page }, workerInfo) => {
+        // @ts-expect-error - TS doesn't know about the "use.e2e" property
+        workerInfo.skip(!workerInfo.project.use?.e2e)
+        const overlays = DuckplayerOverlays.create(page, workerInfo)
+        await overlays.overlaysEnabled({ json: 'overlays-live' })
+        await overlays.userSettingIs('enabled')
+        await overlays.gotoYoutubeHomepage()
+        await page.waitForTimeout(2000)
+        await overlays.clicksFirstShortsThumbnail()
+        await overlays.showsShortsPage()
+    })
 })

integration-test/playwright/duckplayer.spec.js

@@ -28,6 +28,19 @@ test.describe('Duck Player Thumbnail Overlays on YouTube.com', () => {
         await overlays.hoverShort()
         await overlays.overlaysDontShow()
     })
+    /**
+     * https://app.asana.com/0/1201048563534612/1204993915251837/f
+     */
+    test('Clicks are not intercepted on shorts when "enabled"', async ({ page }, workerInfo) => {
+        const overlays = DuckplayerOverlays.create(page, workerInfo)
+        await overlays.overlaysEnabled()
+        await overlays.userSettingIs('enabled')
+        await overlays.gotoThumbsPage()
+        const navigation = overlays.requestWillFail()
+        await overlays.clicksFirstShortsThumbnail()
+        const url = await navigation
+        await overlays.opensShort(url)
+    })
     test('Overlays don\'t show on thumbnails when disabled', async ({ page }, workerInfo) => {
         const overlays = DuckplayerOverlays.create(page, workerInfo)
 

integration-test/playwright/page-objects/duckplayer-overlays.js

@@ -60,6 +60,22 @@ export class DuckplayerOverlays {
         await this.page.getByRole('button', { name: 'Reject the use of cookies and other data for the purposes described' }).click()
     }
 
+    async clicksFirstShortsThumbnail () {
+        await this.page.locator('[href*="/shorts"] img').first().click({ force: true })
+    }
+
+    async showsShortsPage () {
+        await this.page.waitForURL(/^https:\/\/www.youtube.com\/shorts/, { timeout: 5000 })
+    }
+
+    /**
+     * @param {string} requestUrl
+     */
+    opensShort (requestUrl) {
+        const url = new URL(requestUrl)
+        expect(url.pathname).toBe('/shorts/1')
+    }
+
     /**
      * @param {object} [params]
      * @param {"default" | "incremental-dom"} [params.variant]
@@ -348,6 +364,22 @@ export class DuckplayerOverlays {
             ? 'contentScopeScriptsIsolated'
             : 'contentScopeScripts'
     }
+
+    /**
+     * @return {Promise<string>}
+     */
+    requestWillFail () {
+        return new Promise((resolve, reject) => {
+            // on windows it will be a failed request
+            const timer = setTimeout(() => {
+                reject(new Error('timed out'))
+            }, 5000)
+            this.page.on('framenavigated', (req) => {
+                clearTimeout(timer)
+                resolve(req.url())
+            })
+        })
+    }
 }
 
 /**

package.json

@@ -42,7 +42,7 @@
     "playwright-headed": "playwright test --headed",
     "preplaywright": "npm run build-windows && npm run build-apple",
     "preplaywright-headed": "npm run build-windows && npm run build-apple",
-    "playwright-e2e": "E2E=true playwright test --project duckplayer-e2e --headed"
+    "playwright-e2e": "E2E=true playwright test --project duckplayer-e2e"
   },
   "type": "module",
   "workspaces": [

src/features/duckplayer/overlays.js

@@ -109,7 +109,7 @@ export async function initOverlays (environment, comms) {
 
         /**
          * Bind hover events and make sure hovering the video will correctly show the hover
-         * overlay and mousouting will hide it.
+         * overlay and mouse-out will hide it.
          */
         bindEvents: (video) => {
             if (video) {
@@ -119,7 +119,7 @@ export async function initOverlays (environment, comms) {
                     // this can occur with 'shorts'
                     if (video.href !== before) {
                         if (!VideoThumbnail.isSingleVideoURL(video.href)) {
-                            return console.log('bailing because the video has changed')
+                            return
                         }
                     }
                     IconOverlay.moveHoverOverlayToVideoElement(video)
@@ -280,50 +280,25 @@ export async function initOverlays (environment, comms) {
                     // bail if it's not a valid element
                     if (!isValidVideoLinkOrPreview(element)) return
 
-                    // handle mouseover + click events
-                    const handler = {
-                        handleEvent (event) {
-                            switch (event.type) {
-                            case 'mouseover': {
-                                /**
-                                 * Store the element's link value on hover - this occurs just in time
-                                 * before the youtube overlay take sover the event space
-                                 */
-                                const href = element instanceof HTMLAnchorElement
-                                    ? VideoParams.fromHref(element.href)?.toPrivatePlayerUrl()
-                                    : null
-                                if (href) {
-                                    OpenInDuckPlayer.lastMouseOver = href
-                                }
-                                break
-                            }
-                            case 'click': {
-                                /**
-                                 * On click, the receiver might be the preview element - if
-                                 * it is, we want to use the last hovered `a` tag instead
-                                 */
-                                event.preventDefault()
-                                event.stopPropagation()
-
-                                const link = event.target.closest('a')
-                                const fromClosest = VideoParams.fromHref(link?.href)?.toPrivatePlayerUrl()
-
-                                if (fromClosest) {
-                                    comms.openDuckPlayer({ href: fromClosest })
-                                } else if (OpenInDuckPlayer.lastMouseOver) {
-                                    comms.openDuckPlayer({ href: OpenInDuckPlayer.lastMouseOver })
-                                } else {
-                                    // could not navigate, doing nothing
-                                }
-
-                                break
-                            }
-                            }
+                    function handler (event) {
+                        /**
+                         * Opening in Duck Player, preventing normal navigation
+                         */
+                        function openInDuckPlayer (href) {
+                            event.preventDefault()
+                            event.stopPropagation()
+                            comms.openDuckPlayer({ href })
+                        }
+
+                        // select either closest `a` or defer to element.href
+                        const targetValue = event.target.closest('a')?.href || /** @type {HTMLAnchorElement} */(element).href
+                        const validPrivatePlayerUrl = VideoParams.fromHref(targetValue)?.toPrivatePlayerUrl()
+
+                        if (validPrivatePlayerUrl) {
+                            return openInDuckPlayer(validPrivatePlayerUrl)
                         }
                     }
 
-                    // register both handlers
-                    element.addEventListener('mouseover', handler, true)
                     element.addEventListener('click', handler, true)
 
                     // store the handler for removal later (eg: if settings change)
@@ -333,7 +308,6 @@ export async function initOverlays (environment, comms) {
 
         disable: () => {
             OpenInDuckPlayer.clickBoundElements.forEach((handler, element) => {
-                element.removeEventListener('mouseover', handler, true)
                 element.removeEventListener('click', handler, true)
                 OpenInDuckPlayer.clickBoundElements.delete(element)
             })