Skip to content

fix(duckplayer): fix overlays on shorts #603

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 2 commits into from
Jun 29, 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
22 changes: 22 additions & 0 deletions integration-test/playwright/duckplayer.e2e.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { test } from '@playwright/test'
import { DuckplayerOverlays } from './page-objects/duckplayer-overlays.js'

test.describe('e2e: Duck Player Thumbnail Overlays on YouTube.com', () => {
test('e2e: Overlays never appear on "shorts"', 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.gotoYoutubeHomepage()

// Ensure the hover works normally to prevent false positives
await overlays.hoverAYouTubeThumbnail()
await overlays.isVisible()

// now ensure the hover doesn't work on shorts
await overlays.hoverShort()
await overlays.overlaysDontShow()
})
})
13 changes: 13 additions & 0 deletions integration-test/playwright/duckplayer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,19 @@ test.describe('Duck Player Thumbnail Overlays on YouTube.com', () => {
// Then our overlay shows
await overlays.isVisible()
})
test('Overlays never appear on "shorts"', async ({ page }, workerInfo) => {
const overlays = DuckplayerOverlays.create(page, workerInfo)
await overlays.overlaysEnabled()
await overlays.gotoThumbsPage()

// Ensure the hover works normally to prevent false positives
await overlays.hoverAThumbnail()
await overlays.isVisible()

// now ensure the hover doesn't work on shorts
await overlays.hoverShort()
await overlays.overlaysDontShow()
})
test('Overlays don\'t show on thumbnails when disabled', async ({ page }, workerInfo) => {
const overlays = DuckplayerOverlays.create(page, workerInfo)

Expand Down
45 changes: 40 additions & 5 deletions integration-test/playwright/page-objects/duckplayer-overlays.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ export class DuckplayerOverlays {
await this.page.goto(this.overlaysPage)
}

async gotoYoutubeHomepage () {
await this.page.goto('https://www.youtube.com')
// cookie banner
await this.page.getByRole('button', { name: 'Reject the use of cookies and other data for the purposes described' }).click()
}

/**
* @param {object} [params]
* @param {"default" | "incremental-dom"} [params.variant]
Expand Down Expand Up @@ -100,9 +106,14 @@ export class DuckplayerOverlays {
await this.page.getByRole('link', { name: 'Duck Player', exact: true }).waitFor({ state: 'attached' })
}

// Given the "overlays" feature is enabled
async overlaysEnabled () {
await this.setup({ config: loadConfig('overlays') })
/**
* @param {object} [params]
* @param {'overlays' | 'overlays-live'} [params.json="overlays"] - default is settings for localhost
*/
async overlaysEnabled (params = {}) {
const { json = 'overlays' } = params

await this.setup({ config: loadConfig(json) })
}

async serpProxyEnabled () {
Expand Down Expand Up @@ -156,6 +167,16 @@ export class DuckplayerOverlays {
await this.page.locator('.thumbnail[href="/watch?v=1"]').first().hover()
}

async hoverAYouTubeThumbnail () {
await this.page.locator('a.ytd-thumbnail[href^="/watch"]').first().hover({ force: true })
}

async hoverShort () {
// this should auto-wait for our test code to modify the DOM like YouTube does
await this.page.getByRole('heading', { name: 'Shorts', exact: true }).scrollIntoViewIfNeeded()
await this.page.locator('a[href*="/shorts"]').first().hover({ force: true })
}

async clickDDGOverlay () {
await this.hoverAThumbnail()
await this.page.locator('.ddg-play-privately').click({ force: true })
Expand All @@ -166,7 +187,21 @@ export class DuckplayerOverlays {
}

async overlaysDontShow () {
expect(await this.page.locator('.ddg-overlay.ddg-overlay-hover').count()).toEqual(0)
const elements = await this.page.locator('.ddg-overlay.ddg-overlay-hover').count()

// if the element exists, assert that it is hidden
if (elements > 0) {
const style = await this.page.evaluate(() => {
const div = /** @type {HTMLDivElement|null} */(document.querySelector('.ddg-overlay.ddg-overlay-hover'))
if (div) {
return div.style.display
}
return ''
})
expect(style).toEqual('none')
}

// if we get here, the element was absent
}

async watchInDuckPlayer () {
Expand Down Expand Up @@ -316,7 +351,7 @@ export class DuckplayerOverlays {
}

/**
* @param {"overlays"} name
* @param {"overlays" | "overlays-live"} name
* @return {Record<string, any>}
*/
function loadConfig (name) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"unprotectedTemporary": [],
"features": {
"duckPlayer": {
"state": "enabled",
"exceptions": [],
"settings": {
"overlays": {
"youtube": {
"state": "disabled"
},
"serpProxy": {
"state": "disabled"
}
},
"domains": [
{
"domain": "youtube.com",
"patchSettings": [
{
"op": "replace",
"path": "/overlays/youtube/state",
"value": "enabled"
}
]
},
{
"domain": "duckduckgo.com",
"patchSettings": [
{
"op": "replace",
"path": "/overlays/serpProxy/state",
"value": "enabled"
}
]
}
]
}
}
}
}
61 changes: 54 additions & 7 deletions integration-test/test-pages/duckplayer/pages/overlays.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,34 @@
<title>Runtime checks</title>
<link rel="stylesheet" href="../../shared/style.css">
<style>
.thumbnails {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
}
.thumbnail {
height: 100px;
width: 200px;
margin: 5px;
background: lightblue;
border: 1px solid black;
float: left;
aspect-ratio: 16/9;
display: block;
}

.thumbnail img {
.thumbnail img, .short img {
width: 100%;
height: 100%;
object-fit: cover;
display: block;
}

.short {
margin: 5px;
background: lightblue;
border: 1px solid black;
aspect-ratio: 9/16;
display: block;
}

.playlist {
clear: both;
width: 100px;

}
Expand All @@ -37,8 +49,6 @@
}

#loaded {
clear: both;
float: none;
width: 300px;
}

Expand Down Expand Up @@ -81,7 +91,44 @@ <h2>MORE THUMBNAILS</h2>
</div>
</div>

<h2>Shorts</h2>
<div class="thumbnails">
<a class="ytd-thumbnail short" href="/watch?v=1"><img src="thumbnail-dark.jpg"></a>
<a class="ytd-thumbnail short" href="/watch?v=2"><img src="thumbnail-light.jpg"></a>
<a class="ytd-thumbnail short" href="/watch?v=3"><img src="thumbnail-dark.jpg"></a>
<a class="ytd-thumbnail short" href="/watch?v=4"><img src="thumbnail-light.jpg"></a>
<a class="ytd-thumbnail short" href="/watch?v=5"><img src="thumbnail-dark.jpg"></a>
</div>

<div id="loaded"></div>

<script type="module">
const variant = new URLSearchParams(window.location.search).get('variant') || 'default';

const knownVariants = {
"default": () => {
setTimeout(() => {
/**
* This will change '/watch?v=3' links to '/shorts/3'
* It roughly mimics the behaviour of the YouTube Homepage
*/
const shorts = document.querySelectorAll('.short');
shorts.forEach((short) => {
const params = new URLSearchParams(short.search);
short.pathname = "/shorts/" + params.get('v');
short.search = ''
})
}, 200);
},
}

if (variant in knownVariants) {
console.log('executing page variant', variant)
knownVariants[variant]();
} else {
console.warn('variant not found', variant)
}

</script>
</body>
</html>
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
"playwright": "playwright test",
"playwright-headed": "playwright test --headed",
"preplaywright": "npm run build-windows && npm run build-apple",
"preplaywright-headed": "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"
},
"type": "module",
"workspaces": [
Expand Down
7 changes: 7 additions & 0 deletions playwright.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ import { defineConfig, devices } from '@playwright/test'

export default defineConfig({
projects: [
{
name: 'duckplayer-e2e',
testMatch: [
'integration-test/playwright/duckplayer.e2e.spec.js'
],
use: { injectName: 'windows', platform: 'windows', e2e: process.env.E2E }
},
{
name: 'windows',
testMatch: [
Expand Down
2 changes: 1 addition & 1 deletion src/features/duck-player.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* For example, to enable the Duck Player Overlay on YouTube, the following config is used:
*
* ```json
* [[include:integration-test/test-pages/duckplayer/config/overlays.json]]```
* [[include:integration-test/test-pages/duckplayer/config/overlays-live.json]]```
*
*/
import ContentFeature from '../content-feature.js'
Expand Down
8 changes: 8 additions & 0 deletions src/features/duckplayer/overlays.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,15 @@ export async function initOverlays (environment, comms) {
*/
bindEvents: (video) => {
if (video) {
const before = video.href
addTrustedEventListener(video, 'mouseover', () => {
// don't allow the hover overlay to be shown if the video has changed
// this can occur with 'shorts'
if (video.href !== before) {
if (!VideoThumbnail.isSingleVideoURL(video.href)) {
return console.log('bailing because the video has changed')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix - we double-check the video.href to guard against DOM mutations

}
}
IconOverlay.moveHoverOverlayToVideoElement(video)
})

Expand Down