Skip to content

duckplayer fixes #619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions integration-test/playwright/duckplayer.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
13 changes: 13 additions & 0 deletions integration-test/playwright/duckplayer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
32 changes: 32 additions & 0 deletions integration-test/playwright/page-objects/duckplayer-overlays.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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())
})
})
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
62 changes: 18 additions & 44 deletions src/features/duckplayer/overlays.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
})
Expand Down