FEATURE: Use upload:// short URL for videos and audio in composer (#8760)

For consistency this PR introduces using custom markdown and short upload:// URLs for video and audio uploads, rather than just treating them as links and relying on the oneboxer. The markdown syntax for videos is ![file text|video](upload://123456.mp4) and for audio it is ![file text|audio](upload://123456.mp3).

This is achieved in discourse-markdown-it by modifying the rules for images in mardown-it via md.renderer.rules.image. We return HTML instead of the token when we encounter audio or video after | and the preview renders that HTML. Also when uploading an audio or video file we insert the relevant markdown into the composer.
This commit is contained in:
Martin Brennan 2020-01-23 09:41:39 +10:00 committed by GitHub
parent 4646a38ae6
commit 65481858c2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 190 additions and 76 deletions

View File

@ -6,7 +6,7 @@ function isGUID(value) {
);
}
export function imageNameFromFileName(fileName) {
export function markdownNameFromFileName(fileName) {
let name = fileName.substr(0, fileName.lastIndexOf("."));
if (isAppleDevice() && isGUID(name)) {
@ -68,7 +68,7 @@ function validateUploadedFile(file, opts) {
}
if (opts.imagesOnly) {
if (!isAnImage(name) && !isAuthorizedImage(name, staff)) {
if (!isImage(name) && !isAuthorizedImage(name, staff)) {
bootbox.alert(
I18n.t("post.errors.upload_not_authorized", {
authorized_extensions: authorizedImagesExtensions(staff)
@ -193,12 +193,20 @@ export function authorizesOneOrMoreImageExtensions(staff) {
return imagesExtensions(staff).length > 0;
}
export function isAnImage(path) {
export function isImage(path) {
return /\.(png|jpe?g|gif|svg|ico)$/i.test(path);
}
export function isVideo(path) {
return /\.(mov|mp4|webm|m4v|3gp|ogv|avi|mpeg|ogv)$/i.test(path);
}
export function isAudio(path) {
return /\.(mp3|og[ga]|opus|wav|m4[abpr]|aac|flac)$/i.test(path);
}
function uploadTypeFromFileName(fileName) {
return isAnImage(fileName) ? "image" : "attachment";
return isImage(fileName) ? "image" : "attachment";
}
export function allowsImages(staff) {
@ -220,37 +228,33 @@ export function uploadIcon(staff) {
return allowsAttachments(staff) ? "upload" : "far-image";
}
function uploadLocation(url) {
if (Discourse.CDN) {
url = Discourse.getURLWithCDN(url);
return /^\/\//.test(url) ? "http:" + url : url;
} else if (Discourse.S3BaseUrl) {
if (url.indexOf("secure-media-uploads") === -1) {
return "https:" + url;
}
return window.location.protocol + url;
} else {
var protocol = window.location.protocol + "//",
hostname = window.location.hostname,
port = window.location.port ? ":" + window.location.port : "";
return protocol + hostname + port + url;
}
function imageMarkdown(upload) {
return `![${markdownNameFromFileName(upload.original_filename)}|${
upload.thumbnail_width
}x${upload.thumbnail_height}](${upload.short_url || upload.url})`;
}
export function getUploadMarkdown(upload) {
if (isAnImage(upload.original_filename)) {
const name = imageNameFromFileName(upload.original_filename);
return `![${name}|${upload.thumbnail_width}x${
upload.thumbnail_height
}](${upload.short_url || upload.url})`;
} else if (
/\.(mov|mp4|webm|ogv|mp3|ogg|wav|m4a)$/i.test(upload.original_filename)
) {
return uploadLocation(upload.url);
} else {
function playableMediaMarkdown(upload, type) {
return `![${markdownNameFromFileName(upload.original_filename)}|${type}](${
upload.short_url
})`;
}
function attachmentMarkdown(upload) {
return `[${upload.original_filename}|attachment](${
upload.short_url
}) (${I18n.toHumanSize(upload.filesize)})`;
}
export function getUploadMarkdown(upload) {
if (isImage(upload.original_filename)) {
return imageMarkdown(upload);
} else if (isAudio(upload.original_filename)) {
return playableMediaMarkdown(upload, "audio");
} else if (isVideo(upload.original_filename)) {
return playableMediaMarkdown(upload, "video");
} else {
return attachmentMarkdown(upload);
}
}

View File

@ -139,14 +139,43 @@ export function extractDataAttribute(str) {
return [key, value];
}
// videoHTML and audioHTML follow the same HTML syntax
// as oneboxer.rb when dealing with these formats
function videoHTML(token) {
const src = token.attrGet("src");
const origSrc = token.attrGet("data-orig-src");
return `<video width="100%" height="100%" controls>
<source src="${src}" data-orig-src="${origSrc}">
<a href="${src}">${src}</a>
</video>`;
}
function audioHTML(token) {
const src = token.attrGet("src");
const origSrc = token.attrGet("data-orig-src");
return `<audio controls>
<source src="${src}" data-orig-src="${origSrc}">
<a href="${src}">${src}</a>
</audio>`;
}
const IMG_SIZE_REGEX = /^([1-9]+[0-9]*)x([1-9]+[0-9]*)(\s*,\s*(x?)([1-9][0-9]{0,2}?)([%x]?))?$/;
function renderImage(tokens, idx, options, env, slf) {
function renderImageOrPlayableMedia(tokens, idx, options, env, slf) {
const token = tokens[idx];
const alt = slf.renderInlineAsText(token.children, options, env);
const split = alt.split("|");
const altSplit = [];
// markdown-it supports returning HTML instead of continuing to render the current token
// see https://github.com/markdown-it/markdown-it/blob/master/docs/architecture.md#renderer
if (split[1] === "video") {
return videoHTML(token);
} else if (split[1] === "audio") {
return audioHTML(token);
}
// parsing ![myimage|500x300]() or ![myimage|75%]() or ![myimage|500x300, 75%]
for (let i = 0, match, data; i < split.length; ++i) {
if ((match = split[i].match(IMG_SIZE_REGEX)) && match[1] && match[2]) {
let width = match[1];
@ -194,8 +223,11 @@ function renderImage(tokens, idx, options, env, slf) {
return slf.renderToken(tokens, idx, options);
}
function setupImageDimensions(md) {
md.renderer.rules.image = renderImage;
// we have taken over the ![]() syntax in markdown to
// be able to render a video or audio URL as well as the
// image using |video and |audio in the text inside []
function setupImageAndPlayableMediaRenderer(md) {
md.renderer.rules.image = renderImageOrPlayableMedia;
}
function renderAttachment(tokens, idx, options, env, slf) {
@ -319,7 +351,7 @@ export function setup(opts, siteSettings, state) {
setupUrlDecoding(opts.engine);
setupHoister(opts.engine);
setupImageDimensions(opts.engine);
setupImageAndPlayableMediaRenderer(opts.engine);
setupAttachments(opts.engine);
setupBlockBBCode(opts.engine);
setupInlineBBCode(opts.engine);

View File

@ -43,10 +43,17 @@ function rule(state) {
if (mapped) {
token.attrs[srcIndex][1] = mapped.url;
token.attrs.push(["data-base62-sha1", mapped.base62_sha1]);
} else {
// no point putting a transparent .png for audio/video
if (token.content.match(/\|video|\|audio/)) {
token.attrs[srcIndex][1] = state.md.options.discourse.getURL(
"/404"
);
} else {
token.attrs[srcIndex][1] = state.md.options.discourse.getURL(
"/images/transparent.png"
);
}
token.attrs.push(["data-orig-src", origSrc]);
}

View File

@ -39,19 +39,25 @@ export function resetCache() {
_cache = {};
}
function _loadCachedShortUrls($uploads) {
$uploads.each((idx, upload) => {
const $upload = $(upload);
let url;
switch (upload.tagName) {
case "A":
url = lookupCachedUploadUrl($upload.data("orig-href")).short_path;
function retrieveCachedUrl($upload, dataAttribute, callback) {
const cachedUpload = lookupCachedUploadUrl($upload.data(dataAttribute));
const url =
dataAttribute === "orig-href" ? cachedUpload.short_path : cachedUpload.url;
if (url) {
$upload.removeAttr("data-orig-href");
$upload.removeAttr(`data-${dataAttribute}`);
if (url !== MISSING) {
callback(url);
}
}
}
function _loadCachedShortUrls($uploads) {
$uploads.each((_idx, upload) => {
const $upload = $(upload);
switch (upload.tagName) {
case "A":
retrieveCachedUrl($upload, "orig-href", url => {
$upload.attr("href", url);
// Replace "|attachment" with class='attachment'
@ -62,20 +68,29 @@ function _loadCachedShortUrls($uploads) {
$upload.addClass(ATTACHMENT_CSS_CLASS);
$upload.text(content[0]);
}
}
}
});
break;
case "IMG":
url = lookupCachedUploadUrl($upload.data("orig-src")).url;
if (url) {
$upload.removeAttr("data-orig-src");
if (url !== MISSING) {
retrieveCachedUrl($upload, "orig-src", url => {
$upload.attr("src", url);
});
break;
case "SOURCE": // video tag > source tag
retrieveCachedUrl($upload, "orig-src", url => {
$upload.attr("src", url);
if (url.startsWith(`//${window.location.host}`)) {
let hostRegex = new RegExp("//" + window.location.host, "g");
url = url.replace(hostRegex, "");
}
}
$upload.attr("src", window.location.origin + url);
// this is necessary, otherwise because of the src change the
// video just doesn't bother loading!
$upload.parent()[0].load();
});
break;
}
@ -94,7 +109,8 @@ function _loadShortUrls($uploads, ajax) {
}
export function resolveAllShortUrls(ajax) {
const attributes = "img[data-orig-src], a[data-orig-href]";
const attributes =
"img[data-orig-src], a[data-orig-href], source[data-orig-src]";
let $shortUploadUrls = $(attributes);
if ($shortUploadUrls.length > 0) {

View File

@ -132,6 +132,8 @@ export const DEFAULT_LIST = [
"abbr[title]",
"aside.quote",
"aside[data-*]",
"audio",
"audio[controls]",
"b",
"big",
"blockquote",
@ -192,7 +194,14 @@ export const DEFAULT_LIST = [
"strong",
"sub",
"sup",
"source[src]",
"source[data-orig-src]",
"source[type]",
"ul",
"video",
"video[height]",
"video[width]",
"video[controls]",
"ruby",
"ruby[lang]",
"rb",

View File

@ -7,7 +7,7 @@ Dir["#{Rails.root}/lib/onebox/engine/*_onebox.rb"].sort.each { |f| require f }
module Oneboxer
ONEBOX_CSS_CLASS = "onebox"
AUDIO_REGEX = /^\.(mp3|og[ga]|opus|wav|m4[abpr]|aac|flac)$/i
VIDEO_REGEX = /^\.(mov|mp4|m4v|webm|ogv|3gp)$/i
VIDEO_REGEX = /^\.(mov|mp4|webm|m4v|3gp|ogv|avi|mpeg|ogv)$/i
# keep reloaders happy
unless defined? Oneboxer::Result
@ -196,7 +196,6 @@ module Oneboxer
<video width="100%" height="100%" controls="">
<source src='#{url}'>
<a href='#{url}'>#{url}</a>
</source>
</video>
</div>
HTML

View File

@ -973,6 +973,28 @@ QUnit.test("images", assert => {
);
});
QUnit.test("video", assert => {
assert.cooked(
"![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)",
`<p><video width="100%" height="100%" controls>
<source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4">
<a href="/404">/404</a>
</video></p>`,
"It returns the correct video player HTML"
);
});
QUnit.test("audio", assert => {
assert.cooked(
"![young americans|audio](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3)",
`<p><audio controls>
<source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3">
<a href="/404">/404</a>
</audio></p>`,
"It returns the correct audio player HTML"
);
});
QUnit.test("censoring", assert => {
assert.cookedOptions(
"Pleased to meet you, but pleeeease call me later, xyz123",

View File

@ -33,9 +33,22 @@ QUnit.module("lib:pretty-text/upload-short-url", {
}
];
const otherMediaSrcs = [
{
short_url: "upload://d.mp4",
url: "/uploads/default/original/3X/c/b/4.mp4",
short_path: "/uploads/short-url/d.mp4"
},
{
short_url: "upload://e.mp3",
url: "/uploads/default/original/3X/c/b/5.mp3",
short_path: "/uploads/short-url/e.mp3"
}
];
// prettier-ignore
server.post("/uploads/lookup-urls", () => { //eslint-disable-line
return response(imageSrcs.concat(attachmentSrcs));
return response(imageSrcs.concat(attachmentSrcs.concat(otherMediaSrcs)));
});
fixture().html(
@ -79,4 +92,16 @@ QUnit.test("resolveAllShortUrls", async assert => {
url: "/uploads/default/original/3X/c/b/3.pdf",
short_path: "/uploads/short-url/c.pdf"
});
lookup = lookupCachedUploadUrl("upload://d.mp4");
assert.deepEqual(lookup, {
url: "/uploads/default/original/3X/c/b/4.mp4",
short_path: "/uploads/short-url/d.mp4"
});
lookup = lookupCachedUploadUrl("upload://e.mp3");
assert.deepEqual(lookup, {
url: "/uploads/default/original/3X/c/b/5.mp3",
short_path: "/uploads/short-url/e.mp3"
});
});

View File

@ -1,7 +1,7 @@
import {
validateUploadedFiles,
authorizedExtensions,
isAnImage,
isImage,
allowsImages,
allowsAttachments,
getUploadMarkdown
@ -122,18 +122,18 @@ QUnit.test("allows valid uploads to go through", assert => {
assert.not(bootbox.alert.calledOnce);
});
QUnit.test("isAnImage", assert => {
QUnit.test("isImage", assert => {
["png", "jpg", "jpeg", "gif", "ico"].forEach(extension => {
var image = "image." + extension;
assert.ok(isAnImage(image), image + " is recognized as an image");
assert.ok(isImage(image), image + " is recognized as an image");
assert.ok(
isAnImage("http://foo.bar/path/to/" + image),
isImage("http://foo.bar/path/to/" + image),
image + " is recognized as an image"
);
});
assert.not(isAnImage("file.txt"));
assert.not(isAnImage("http://foo.bar/path/to/file.txt"));
assert.not(isAnImage(""));
assert.not(isImage("file.txt"));
assert.not(isImage("http://foo.bar/path/to/file.txt"));
assert.not(isImage(""));
});
QUnit.test("allowsImages", assert => {