fix(duck-player): capture the correct url before a click occurs (#579)

Co-authored-by: Shane Osbourne <[email protected]>
duckduckgo · Jun 16, 2023 · ea5b5fc · ea5b5fc
1 parent f41a73c
commit ea5b5fc
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 22 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -6,3 +6,7 @@ integration-test/extension/contentScope.js
 integration-test/pages/build
 packages/special-pages/pages/**/public
 script-overload-snapshots/
+packages/special-pages/playwright-report/
+packages/special-pages/test-results/
+/playwright-report
+/test-results
diff --git a/src/features/duckplayer/overlays.js b/src/features/duckplayer/overlays.js
@@ -252,7 +252,8 @@ export async function initOverlays (environment, comms) {
     const OpenInDuckPlayer = {
         clickBoundElements: new Map(),
         enabled: false,
-
+        /** @type {string|null} */
+        lastMouseOver: null,
         bindEventsToAll: () => {
             if (!OpenInDuckPlayer.enabled) {
                 return
@@ -263,38 +264,69 @@ export async function initOverlays (environment, comms) {
                 return VideoThumbnail.isSingleVideoURL(element?.getAttribute('href')) ||
                     element.getAttribute('id') === 'media-container-link'
             }
-            const excludeAlreadyBound = (element) => !OpenInDuckPlayer.clickBoundElements.has(element)
-
             videoLinksAndPreview
-                .filter(excludeAlreadyBound)
-                .forEach(element => {
-                    if (isValidVideoLinkOrPreview(element)) {
-                        const onClickOpenDuckPlayer = (event) => {
-                            event.preventDefault()
-                            event.stopPropagation()
-
-                            const link = event.target.closest('a')
-
-                            if (link) {
-                                const href = VideoParams.fromHref(link.href)?.toPrivatePlayerUrl()
+                .forEach((/** @type {HTMLElement|HTMLAnchorElement} */element) => {
+                    // bail when this element was already seen
+                    if (OpenInDuckPlayer.clickBoundElements.has(element)) return
+
+                    // 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) {
-                                    comms.openDuckPlayer({ 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
+                                }
 
-                            return false
+                                break
+                            }
+                            }
                         }
+                    }
 
-                        element.addEventListener('click', onClickOpenDuckPlayer, true)
+                    // register both handlers
+                    element.addEventListener('mouseover', handler, true)
+                    element.addEventListener('click', handler, true)
 
-                        OpenInDuckPlayer.clickBoundElements.set(element, onClickOpenDuckPlayer)
-                    }
+                    // store the handler for removal later (eg: if settings change)
+                    OpenInDuckPlayer.clickBoundElements.set(element, handler)
                 })
         },
 
         disable: () => {
-            OpenInDuckPlayer.clickBoundElements.forEach((functionToRemove, element) => {
-                element.removeEventListener('click', functionToRemove, true)
+            OpenInDuckPlayer.clickBoundElements.forEach((handler, element) => {
+                element.removeEventListener('mouseover', handler, true)
+                element.removeEventListener('click', handler, true)
                 OpenInDuckPlayer.clickBoundElements.delete(element)
             })
 

diff --git a/src/features/duckplayer/util.js b/src/features/duckplayer/util.js
@@ -200,7 +200,13 @@ export class VideoParams {
      * @returns {VideoParams|null}
      */
     static fromHref (href) {
-        const url = new URL(href)
+        let url
+        try {
+            url = new URL(href)
+        } catch (e) {
+            return null
+        }
+
         const vParam = url.searchParams.get('v')
         const tParam = url.searchParams.get('t')
 

diff --git a/tsconfig.json b/tsconfig.json
@@ -26,6 +26,8 @@
     "integration-test/pages",
     "integration-test/extension",
     "packages/special-pages/pages/**/public",
+    "packages/special-pages/playwright-report",
+    "packages/special-pages/test-results",
     "docs"
   ]
 }