Skip to content

Commit

Permalink
feat: add canonical tag to pages with pagination
Browse files Browse the repository at this point in the history
  • Loading branch information
kontrollanten committed Sep 24, 2024
1 parent a85a48f commit b620bea
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 10 deletions.
40 changes: 36 additions & 4 deletions packages/tests/src/client/index-html.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-unused-expressions,@typescript-eslint/require-await */

import { expect } from 'chai'
import { HttpStatusCode, VideoPlaylistCreateResult } from '@peertube/peertube-models'
import { HttpStatusCode, ServerConfig, VideoPlaylistCreateResult } from '@peertube/peertube-models'
import { cleanupTests, makeGetRequest, makeHTMLRequest, PeerTubeServer } from '@peertube/peertube-server-commands'
import { checkIndexTags, getWatchPlaylistBasePaths, getWatchVideoBasePaths, prepareClientTests } from '@tests/shared/client.js'

Expand All @@ -22,6 +22,8 @@ describe('Test index HTML generation', function () {

let instanceDescription: string

const getTitleWithSuffix = (title: string, config: ServerConfig) => `${title} - ${config.instance.name}`

before(async function () {
this.timeout(120000);

Expand All @@ -46,7 +48,7 @@ describe('Test index HTML generation', function () {
const config = await servers[0].config.getConfig()
const res = await makeHTMLRequest(servers[0].url, '/videos/trending')

checkIndexTags(res.text, 'PeerTube', instanceDescription, '', config)
checkIndexTags(res.text, getTitleWithSuffix('Trending', config), instanceDescription, '', config)
})

it('Should update the customized configuration and have the correct index html tags', async function () {
Expand All @@ -70,20 +72,25 @@ describe('Test index HTML generation', function () {
const config = await servers[0].config.getConfig()
const res = await makeHTMLRequest(servers[0].url, '/videos/trending')

checkIndexTags(res.text, 'PeerTube updated', 'my short description', 'body { background-color: red; }', config)
checkIndexTags(res.text, getTitleWithSuffix('Trending', config), 'my short description', 'body { background-color: red; }', config)
})

it('Should have valid index html updated tags (title, description...)', async function () {
const config = await servers[0].config.getConfig()
const res = await makeHTMLRequest(servers[0].url, '/videos/trending')

checkIndexTags(res.text, 'PeerTube updated', 'my short description', 'body { background-color: red; }', config)
checkIndexTags(res.text, getTitleWithSuffix('Trending', config), 'my short description', 'body { background-color: red; }', config)
})
})

describe('Canonical tags', function () {

it('Should use the original video URL for the canonical tag', async function () {
const res = await makeHTMLRequest(servers[0].url, '/videos/trending?page=2')
expect(res.text).to.contain(`<link rel="canonical" href="${servers[0].url}/videos/trending?page=2" />`)
})

it('Should use pagination in video URL for the canonical tag', async function () {
for (const basePath of getWatchVideoBasePaths()) {
for (const id of videoIds) {
const res = await makeHTMLRequest(servers[0].url, basePath + id)
Expand Down Expand Up @@ -111,6 +118,18 @@ describe('Test index HTML generation', function () {
accountURLtest(await makeHTMLRequest(servers[0].url, '/@root@' + servers[0].host))
})

it('Should use pagination in account video channels URL for the canonical tag', async function () {
const res = await makeHTMLRequest(servers[0].url, '/a/root/video-channels?page=2')

expect(res.text).to.contain(`<link rel="canonical" href="${servers[0].url}/a/root/video-channels?page=2" />`)
})

it('Should use pagination in account videos URL for the canonical tag', async function () {
const res = await makeHTMLRequest(servers[0].url, '/a/root/videos?page=2')

expect(res.text).to.contain(`<link rel="canonical" href="${servers[0].url}/a/root/videos?page=2" />`)
})

it('Should use the original channel URL for the canonical tag', async function () {
const channelURLtests = res => {
expect(res.text).to.contain(`<link rel="canonical" href="${servers[0].url}/c/root_channel/videos" />`)
Expand All @@ -120,6 +139,19 @@ describe('Test index HTML generation', function () {
channelURLtests(await makeHTMLRequest(servers[0].url, '/c/root_channel@' + servers[0].host))
channelURLtests(await makeHTMLRequest(servers[0].url, '/@root_channel@' + servers[0].host))
})

it('Should use pagination in channel videos URL for the canonical tag', async function () {
const res = await makeHTMLRequest(servers[0].url, '/c/root_channel/videos?page=2')

expect(res.text).to.contain(`<link rel="canonical" href="${servers[0].url}/c/root_channel/videos?page=2" />`)
})

it('Should use pagination in channel playlists URL for the canonical tag', async function () {
const res = await makeHTMLRequest(servers[0].url, '/c/root_channel/video-playlists?page=2')
console.log(res.text)

expect(res.text).to.contain(`<link rel="canonical" href="${servers[0].url}/c/root_channel/video-playlists?page=2" />`)
})
})

describe('Indexation tags', function () {
Expand Down
14 changes: 14 additions & 0 deletions server/core/controllers/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { currentDir, root } from '@peertube/peertube-node-utils'
import { STATIC_MAX_AGE } from '../initializers/constants.js'
import { ClientHtml, sendHTML, serveIndexHTML } from '../lib/html/client-html.js'
import { asyncMiddleware, buildRateLimiter, embedCSP } from '../middlewares/index.js'
import { VideosOrderType } from '../lib/html/shared/videos-html.js'

const clientsRouter = express.Router()

Expand All @@ -29,6 +30,11 @@ clientsRouter.use([ '/w/p/:id', '/videos/watch/playlist/:id' ],
asyncMiddleware(generateWatchPlaylistHtmlPage)
)

clientsRouter.get([ '/videos/:type(overview|trending|recently-added|local)', '/' ],
clientsRateLimiter,
asyncMiddleware(generateVideosHtmlPage)
)

clientsRouter.use([ '/w/:id', '/videos/watch/:id' ],
clientsRateLimiter,
asyncMiddleware(generateWatchHtmlPage)
Expand Down Expand Up @@ -186,6 +192,14 @@ async function generateVideoPlaylistEmbedHtmlPage (req: express.Request, res: ex
return sendHTML(html, res)
}

async function generateVideosHtmlPage (req: express.Request, res: express.Response) {
const { type } = req.params as { type: VideosOrderType }

const html = await ClientHtml.getVideosHTMLPage(type, req, res, req.params.language)

return sendHTML(html, res, true)
}

async function generateWatchHtmlPage (req: express.Request, res: express.Response) {
// Thread link is '/w/:videoId;threadId=:threadId'
// So to get the videoId we need to remove the last part
Expand Down
90 changes: 90 additions & 0 deletions server/core/controllers/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1">

<meta name="theme-color" content="#fff" />
<meta property="og:platform" content="PeerTube" />
<!-- Web Manifest file -->
<link rel="manifest" href="/manifest.webmanifest?[manifestContentHash]">

<link rel="icon" type="image/png" href="/client/assets/images/favicon.png?[faviconContentHash]" />
<link rel="apple-touch-icon" href="/client/assets/images/icons/icon-144x144.png" sizes="144x144" />
<link rel="apple-touch-icon" href="/client/assets/images/icons/icon-192x192.png" sizes="192x192" />

<!-- logo background-image -->
<style type="text/css">
.icon-logo {
background-image: url(/client/assets/images/logo.svg?[logoContentHash]);
}
</style>

<!-- base url -->
<base href="/">

<!-- /!\ The following comment is used by the server to prerender some tags /!\ -->

<!-- title tag -->
<!-- description tag -->
<!-- custom css tag -->
<!-- meta tags -->
<!-- server config -->

<!-- /!\ Do not remove it /!\ -->
</head>

<!-- 3. Display the application -->
<body id="custom-css">

<noscript class="alert alert-light">
<h1 class="alert-heading">PeerTube</h1>
<h2 class="mb-3">JavaScript required</h2>

<p>It seems JavaScript is either blocked or disabled in your web browser. We totally get that. However, this page will not work without it.</p>
<p>If you are concerned about the security and privacy (or lack thereof) of JavaScript web applications, you might want to review the source code of the instance you are trying to access, or look for security audits.</p>

<hr>

<h2 class="mb-3">Your options</h2>

<ul>
<li>Allow JavaScript in your browser</li>
<li>Use one of the <a class="link-orange" href="https://docs.joinpeertube.org/use/third-party-application" target="_blank">third-party applications</a> to browse this instance</li>
<li>Review the source code on <a class="link-orange" href="https://github.com/Chocobozzz/PeerTube" target="_blank">GitHub</a> or <a class="link-orange" href="https://framagit.org/framasoft/peertube/PeerTube" target="_blank">Framasoft's GitLab</a>, and ask for modifications from the instance owner.
</ul>
</noscript>

<div id="incompatible-browser" class="alert alert-light" role="alert" style="display: none">
<h1 class="alert-heading">PeerTube</h1>
<h2 class="mb-3">Incompatible browser</h2>

<p>We are sorry but it seems that PeerTube is not compatible with your web browser.</p>

<hr>
<p>Please try with the latest version of <a class="link-orange" href="https://www.mozilla.org" target="_blank">Mozilla Firefox</a>.</p>
<p class="mb-0">If you think this is a mistake, please <a class="link-orange" href="https://github.com/Chocobozzz/PeerTube/issues/new" target="_blank">report it</a>.</p>
</div>

<script type="text/javascript">
function displayIncompatibleBrowser () {
var elem = document.getElementById('incompatible-browser')
if (elem.className.indexOf('browser-ok') === -1) {
elem.style.display = 'block'
}
}

window.onerror = function () {
displayIncompatibleBrowser()
}

if (/MSIE|Trident/.test(window.navigator.userAgent) ) {
displayIncompatibleBrowser()
}
</script>

<my-app>
</my-app>

</body>
</html>
16 changes: 16 additions & 0 deletions server/core/lib/html/client-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { VideoHtml } from './shared/video-html.js'
import { PlaylistHtml } from './shared/playlist-html.js'
import { ActorHtml } from './shared/actor-html.js'
import { PageHtml } from './shared/page-html.js'
import { VideosHtml, VideosOrderType } from './shared/videos-html.js'
import { CONFIG } from '@server/initializers/config.js'

class ClientHtml {

Expand All @@ -19,6 +21,20 @@ class ClientHtml {

// ---------------------------------------------------------------------------

static getVideosHTMLPage (type: VideosOrderType, req: express.Request, res: express.Response, paramLang?: string) {
if (type) {
return VideosHtml.getVideosHTML(type, req, res)
}

const [ , eventualType ] = CONFIG.INSTANCE.DEFAULT_CLIENT_ROUTE.split('/videos/') as VideosOrderType[]

if (eventualType) {
return VideosHtml.getVideosHTML(eventualType, req, res)
}

return PageHtml.getDefaultHTML(req, res, paramLang)
}

static getWatchHTMLPage (videoIdArg: string, req: express.Request, res: express.Response) {
return VideoHtml.getWatchVideoHTML(videoIdArg, req, res)
}
Expand Down
20 changes: 19 additions & 1 deletion server/core/lib/html/shared/actor-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,25 @@ export class ActorHtml {
let customHTML = TagsHtml.addTitleTag(html, entity.getDisplayName())
customHTML = TagsHtml.addDescriptionTag(customHTML, escapedTruncatedDescription)

const url = entity.getClientUrl()
const eventualPage = req.path.split('/').pop()
let url

if (entity instanceof AccountModel) {
const page = [ 'video-channels', 'videos' ].includes(eventualPage)
? eventualPage
: undefined
url = entity.getClientUrl(page as 'video-channels' | 'videos')
} else if (entity instanceof VideoChannelModel) {
const page = [ 'video-playlists', 'videos' ].includes(eventualPage)
? eventualPage
: undefined
url = entity.getClientUrl(page as 'video-playlists' | 'videos')
}

if (req.query.page) {
url += `?page=${req.query.page}`
}

const siteName = CONFIG.INSTANCE.NAME
const title = entity.getDisplayName()

Expand Down
1 change: 0 additions & 1 deletion server/core/lib/html/shared/video-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { PageHtml } from './page-html.js'
import { TagsHtml } from './tags-html.js'

export class VideoHtml {

static async getWatchVideoHTML (videoIdArg: string, req: express.Request, res: express.Response) {
const videoId = toCompleteUUID(videoIdArg)

Expand Down
52 changes: 52 additions & 0 deletions server/core/lib/html/shared/videos-html.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { escapeHTML } from '@peertube/peertube-core-utils'
import express from 'express'
import { CONFIG } from '../../../initializers/config.js'
import { WEBSERVER } from '../../../initializers/constants.js'
import { PageHtml } from './page-html.js'
import { TagsHtml } from './tags-html.js'

export type VideosOrderType = 'local' | 'trending' | 'overview' | 'recently-added'

export class VideosHtml {

static async getVideosHTML (type: VideosOrderType, req: express.Request, res: express.Response) {
const html = await PageHtml.getIndexHTML(req, res)

return this.buildVideosHTML({
html,
type,
currentPage: req.query.page
})
}

// ---------------------------------------------------------------------------
// Private
// ---------------------------------------------------------------------------

private static buildVideosHTML (options: {
html: string
type: VideosOrderType
currentPage: string
}) {
const { html, currentPage, type } = options

const title = type === 'recently-added' ? 'Recently added' : type.slice(0, 1).toUpperCase() + type.slice(1)
let customHTML = TagsHtml.addTitleTag(html, title)
customHTML = TagsHtml.addDescriptionTag(customHTML)

let url = WEBSERVER.URL + '/videos/' + type

if (currentPage) {
url += `?page=${currentPage}`
}

return TagsHtml.addTags(customHTML, {
url,

escapedSiteName: escapeHTML(CONFIG.INSTANCE.NAME),
escapedTitle: title,

indexationPolicy: 'always'
}, {})
}
}
4 changes: 2 additions & 2 deletions server/core/models/account/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,8 +476,8 @@ export class AccountModel extends SequelizeModel<AccountModel> {
}

// Avoid error when running this method on MAccount... | MChannel...
getClientUrl (this: MAccountHost | MChannelHost) {
return WEBSERVER.URL + '/a/' + this.Actor.getIdentifier() + '/video-channels'
getClientUrl (this: MAccountHost | MChannelHost, page: 'video-channels' | 'videos' = 'video-channels') {
return WEBSERVER.URL + '/a/' + this.Actor.getIdentifier() + '/' + page
}

isBlocked () {
Expand Down
4 changes: 2 additions & 2 deletions server/core/models/video/video-channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -841,8 +841,8 @@ export class VideoChannelModel extends SequelizeModel<VideoChannelModel> {
}

// Avoid error when running this method on MAccount... | MChannel...
getClientUrl (this: MAccountHost | MChannelHost) {
return WEBSERVER.URL + '/c/' + this.Actor.getIdentifier() + '/videos'
getClientUrl (this: MAccountHost | MChannelHost, page: 'video-playlists' | 'videos' = 'videos') {
return WEBSERVER.URL + '/c/' + this.Actor.getIdentifier() + '/' + page
}

getDisplayName () {
Expand Down

0 comments on commit b620bea

Please sign in to comment.