Update how media player's iframe src is constructed

If the file data has and embedded_iframe_url, use it but add
a "type" url param.

If the file data has no embedded_iframe_url, but does have a
media_entry_id, use it to construct the url.

If there is no embedded_iframe_url or media_entry_id,
the mediahref url param is no longer
url encoded and includes only the files pathname.

In the last case, if the current URL includes a verifier param,
media_player_iframe_content copies it to the mediahref's value before
handing it to CanvasMediaPlayer.

closes LS-1501
flag=rce_enhancements

test plan
  - in the RCE upload or record a video
  > expect the player's iframe src to look like
    /media_objects_iframe/<the-media-id>?type=video
  - select a video from Course or User Media tray
  > expect the player's iframe src to look like
    /media_objects_iframe/<the-media-id>?type=video
  - upload a video file via Documents > Upload Document
  > expect the player's iframe src to look like
    /media_objects_iframe?mediahref=/courses/1/files/17?type=video
  - if you upload a file from Upload Documents as a student
  > expect the iframe src to include the file verifier
  - save
  > expect all the videos to be playable
  > expect the student video to be playable by another student

  - do it all again with audio
  > expect to get the audio player (it will have a mucical notes
    icon)

Change-Id: I632583b5238aae85445ffb88a38f09a79f749f18
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/248774
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Ed Schiebel 2020-09-28 13:36:03 -04:00
parent 19e5531108
commit 77b361e382
11 changed files with 123 additions and 155 deletions

View File

@ -18,25 +18,33 @@
import React from 'react'
import ReactDOM from 'react-dom'
import {parse} from 'url'
import ready from '@instructure/ready'
import CanvasMediaPlayer from '../shared/media/CanvasMediaPlayer'
import closedCaptionLanguages from '../shared/closedCaptionLanguages'
ready(() => {
// get the media_id from something like
// `http://canvas.example.com/media_objects_iframe/m-48jGWTHdvcV5YPdZ9CKsqbtRzu1jURgu`
// `http://canvas.example.com/media_objects_iframe/m-48jGWTHdvcV5YPdZ9CKsqbtRzu1jURgu?type=video`
// or
// `http://canvas.example.com/media_objects_iframe/?href=http://url/to/file.mov`
// `http://canvas.example.com/media_objects_iframe/?type=video&mediahref=url/to/file.mov`
const media_id = window.location.pathname.split('media_objects_iframe/').pop()
const media_href_match = window.location.search.match(/mediahref=([^&]+)/)
const is_audio = /type=audio/.test(window.location.search)
const media_object = ENV.media_object || {}
const parsed_loc = parse(window.location.href, true)
const is_video =
/video/.test(media_object?.media_type) || /type=video/.test(window.location.search)
let href_source
if (media_href_match) {
if (is_audio) {
href_source = decodeURIComponent(media_href_match[1])
} else {
href_source = [decodeURIComponent(media_href_match[1])]
href_source = decodeURIComponent(media_href_match[1])
if (parsed_loc.query.verifier) {
const delim = href_source.indexOf('?') > 0 ? '&' : '?'
href_source = `${href_source}${delim}verifier=${parsed_loc.query.verifier}`
}
if (is_video) {
href_source = [href_source]
}
}
@ -47,14 +55,13 @@ ready(() => {
const div = document.body.firstElementChild
if (!window.frameElement) {
// we're standalone
if (is_audio) {
div.setAttribute('style', 'width: 320px; height: 14.25rem; margin: 1rem auto;')
} else {
if (is_video) {
// CanvasMediaPlayer leaves room for the 16px vertical margin.
div.setAttribute('style', 'width: 640px; max-width: 100%; margin: 16px auto;')
} else {
div.setAttribute('style', 'width: 320px; height: 14.25rem; margin: 1rem auto;')
}
}
const media_object = ENV.media_object || {}
const mediaTracks = media_object?.media_tracks.map(track => {
return {
@ -70,7 +77,7 @@ ready(() => {
media_id={media_id}
media_sources={href_source || media_object.media_sources}
media_tracks={mediaTracks}
type={is_audio ? 'audio' : 'video'}
type={is_video ? 'video' : 'audio'}
/>,
document.getElementById('player_container')
)

View File

@ -18,12 +18,11 @@
export function fileEmbed(file) {
const fileMimeClass = mimeClass(file)
const fileMediaEntryId = mediaEntryId(file)
if (fileMimeClass === 'image') {
return {type: 'image'}
} else if (fileMimeClass === 'video' || fileMimeClass === 'audio') {
return {type: fileMimeClass, id: fileMediaEntryId}
return {type: fileMimeClass}
} else if (file.preview_url) {
return {type: 'scribd'}
} else {
@ -31,10 +30,6 @@ export function fileEmbed(file) {
}
}
function mediaEntryId(file) {
return file.media_entry_id || 'maybe'
}
export function mimeClass(file) {
if (file.mime_class) {
return file.mime_class

View File

@ -417,7 +417,7 @@ describe('contentInsertion', () => {
const audio = audioFromTray()
const result = contentInsertion.insertAudio(editor, audio)
expect(editor.insertContent).toHaveBeenCalledWith(
'<iframe data-media-id="29" data-media-type="audio" src="/media_objects_iframe?mediahref=url%2Fto%2Fcourse%2Ffile&type=audio" style="width:320px;height:14.25rem;display:inline-block;" title="Audio player for filename.mp3"></iframe>&nbsp;'
'<iframe data-media-id="29" data-media-type="audio" src="/media_objects_iframe?mediahref=url/to/course/file&type=audio" style="width:320px;height:14.25rem;display:inline-block;" title="Audio player for filename.mp3"></iframe>&nbsp;'
)
expect(result).toEqual('the inserted iframe')
})

View File

@ -169,54 +169,42 @@ describe('contentRendering', () => {
})
describe('renderVideo', () => {
it('builds iframe src from tray video data', () => {
it('builds html from tray video data', () => {
const video = videoFromTray()
const src = contentRendering.mediaIframeSrcFromFile(video)
expect(src).toEqual('/media_objects_iframe/17?type=video')
})
it('builds iframe src from uploaded video data', () => {
const video = videoFromUpload()
const src = contentRendering.mediaIframeSrcFromFile(video)
expect(src).toEqual('/url/to/m-media-id?type=video')
})
it('builds the html from tray video data', () => {
const video = videoFromTray()
const rendered = contentRendering.renderVideo(video)
expect(rendered).toEqual(
'<iframe allow="fullscreen" allowfullscreen data-media-id="17" data-media-type="video" src="/media_objects_iframe/17?type=video" style="width:400px;height:225px;display:inline-block;" title="Video player for filename.mov"></iframe>&nbsp;'
const html = contentRendering.renderVideo(video)
expect(html).toEqual(
`<iframe allow="fullscreen" allowfullscreen data-media-id="17" data-media-type="video" src="${video.embedded_iframe_url}?type=video" style="width:400px;height:225px;display:inline-block;" title="Video player for filename.mov"></iframe>&nbsp;`
)
})
it('builds the html from uploaded video data', () => {
it('builds html from uploaded video data', () => {
const video = videoFromUpload()
const rendered = contentRendering.renderVideo(video)
const html = contentRendering.renderVideo(video)
expect(html).toEqual(
`<iframe allow="fullscreen" allowfullscreen data-media-id="m-media-id" data-media-type="video" src="${video.embedded_iframe_url}?type=video" style="width:400px;height:225px;display:inline-block;" title="Video player for filename.mov"></iframe>&nbsp;`
)
})
expect(rendered).toEqual(
'<iframe allow="fullscreen" allowfullscreen data-media-id="m-media-id" data-media-type="video" src="/url/to/m-media-id?type=video" style="width:400px;height:225px;display:inline-block;" title="Video player for filename.mov"></iframe>&nbsp;'
it('builds html from canvas file data', () => {
const file = {
id: '17',
url: '/files/17',
title: 'filename.mov',
type: 'video'
}
const html = contentRendering.renderVideo(file)
expect(html).toEqual(
'<iframe allow="fullscreen" allowfullscreen data-media-id="17" data-media-type="video" src="/media_objects_iframe?mediahref=/files/17&type=video" style="width:400px;height:225px;display:inline-block;" title="Video player for filename.mov"></iframe>&nbsp;'
)
})
})
describe('renderAudio', () => {
it('builds iframe src from tray audio data', () => {
const audio = audioFromTray()
const src = contentRendering.mediaIframeSrcFromFile(audio)
expect(src).toEqual('/media_objects_iframe?mediahref=url%2Fto%2Fcourse%2Ffile&type=audio')
})
it('builds iframe src from uploaded audio data', () => {
const audio = audioFromUpload()
const src = contentRendering.mediaIframeSrcFromFile(audio)
expect(src).toEqual('/url/to/m-media-id?type=audio')
})
it('builds the html from tray audio data', () => {
const audio = audioFromTray()
const rendered = contentRendering.renderAudio(audio)
expect(rendered).toEqual(
'<iframe data-media-id="29" data-media-type="audio" src="/media_objects_iframe?mediahref=url%2Fto%2Fcourse%2Ffile&type=audio" style="width:320px;height:14.25rem;display:inline-block;" title="Audio player for filename.mp3"></iframe>&nbsp;'
'<iframe data-media-id="29" data-media-type="audio" src="/media_objects_iframe?mediahref=url/to/course/file&type=audio" style="width:320px;height:14.25rem;display:inline-block;" title="Audio player for filename.mp3"></iframe>&nbsp;'
)
})
@ -227,5 +215,18 @@ describe('contentRendering', () => {
'<iframe data-media-id="m-media-id" data-media-type="audio" src="/url/to/m-media-id?type=audio" style="width:320px;height:14.25rem;display:inline-block;" title="Audio player for filename.mp3"></iframe>&nbsp;'
)
})
it('builds html from canvas file data', () => {
const file = {
id: '17',
url: '/files/17',
title: 'filename.mp3',
type: 'audio'
}
const html = contentRendering.renderAudio(file)
expect(html).toEqual(
'<iframe data-media-id="17" data-media-type="audio" src="/media_objects_iframe?mediahref=/files/17&type=audio" style="width:320px;height:14.25rem;display:inline-block;" title="Audio player for filename.mp3"></iframe>&nbsp;'
)
})
})
})

View File

@ -17,13 +17,7 @@
*/
import classnames from 'classnames'
import {
renderImage,
renderLinkedImage,
renderVideo,
renderAudio,
mediaIframeSrcFromFile
} from './contentRendering'
import {renderImage, renderLinkedImage, renderVideo, renderAudio} from './contentRendering'
import scroll from '../common/scroll'
import {
cleanUrl,
@ -31,6 +25,7 @@ import {
isOnlyTextSelected,
isImageFigure
} from './contentInsertionUtils'
import {mediaPlayerURLFromFile} from './plugins/shared/fileTypeUtils'
/** * generic content insertion ** */
@ -252,7 +247,7 @@ export function insertVideo(editor, video) {
// video iframe. Look for the iframe with the right
// src attribute. (Aside: tinymce strips the id or data-*
// attributes from the iframe, that's why we can't look for those)
const src = mediaIframeSrcFromFile(video)
const src = mediaPlayerURLFromFile(video)
result = result.querySelector(`iframe[src="${src}"]`)
// When the iframe is inserted, it doesn't allow the video to play
@ -264,14 +259,14 @@ export function insertVideo(editor, video) {
return result
} else {
return insertLink(editor, {...video, href: mediaIframeSrcFromFile(video)})
return insertLink(editor, {...video, href: mediaPlayerURLFromFile(video)})
}
}
export function insertAudio(editor, audio) {
if (editor.selection.isCollapsed()) {
let result = insertContent(editor, renderAudio(audio))
const src = mediaIframeSrcFromFile(audio)
const src = mediaPlayerURLFromFile(audio)
result = result.querySelector(`iframe[src="${src}"]`)
// When the iframe is inserted, it doesn't allow the audio to play
@ -283,6 +278,6 @@ export function insertAudio(editor, audio) {
return result
} else {
return insertLink(editor, {...audio, href: mediaIframeSrcFromFile(audio)})
return insertLink(editor, {...audio, href: mediaPlayerURLFromFile(audio)})
}
}

View File

@ -24,7 +24,7 @@ import {
VIDEO_SIZE_DEFAULT,
AUDIO_PLAYER_SIZE
} from './plugins/instructure_record/VideoOptionsTray/TrayController'
import {isAudio} from './plugins/shared/fileTypeUtils'
import {mediaPlayerURLFromFile} from './plugins/shared/fileTypeUtils'
import {prepEmbedSrc, prepLinkedSrc} from '../common/fileUrl'
export function renderLink(data, contents) {
@ -100,16 +100,8 @@ export function renderImage(image, opts) {
return renderToStaticMarkup(constructJSXImageElement(image, opts))
}
export function mediaIframeSrcFromFile(fileProps) {
const type = isAudio(fileProps.content_type || fileProps.type) ? 'audio' : 'video'
if (fileProps.embedded_iframe_url) {
return `${fileProps.embedded_iframe_url}?type=${type}`
}
return `/media_objects_iframe?mediahref=${encodeURIComponent(fileProps.href)}&type=${type}`
}
export function renderVideo(video) {
const src = mediaIframeSrcFromFile(video)
const src = mediaPlayerURLFromFile(video)
return `
<iframe
allow="fullscreen"
@ -129,7 +121,7 @@ export function renderVideo(video) {
}
export function renderAudio(audio) {
const src = mediaIframeSrcFromFile(audio)
const src = mediaPlayerURLFromFile(audio)
return `
<iframe
data-media-id="${audio.media_id || audio.id || audio.file_id}"

View File

@ -22,7 +22,7 @@ import classnames from 'classnames'
import {View} from '@instructure/ui-layout'
import {mediaObjectShape} from './fileShape'
import {downloadToWrap} from '../../../common/fileUrl'
import {embedded_iframe_url_fromFile} from './fileTypeUtils'
import {mediaPlayerURLFromFile} from './fileTypeUtils'
// TODO: should find a better way to share this code
import FileBrowser from '../../../canvasFileBrowser/FileBrowser'
@ -66,7 +66,7 @@ export default function RceFileBrowser(props) {
instructure_scribd_file: canPreview
})
const url = downloadToWrap(fileInfo.api.url)
const embedded_iframe_url = embedded_iframe_url_fromFile(fileInfo.api)
const embedded_iframe_url = mediaPlayerURLFromFile(fileInfo.api)
onFileSelect({
name: fileInfo.name,

View File

@ -22,8 +22,7 @@ import {
isVideo,
isAudio,
isText,
mediaFileUrlToEmbeddedIframeUrl,
embedded_iframe_url_fromFile
mediaPlayerURLFromFile
} from '../fileTypeUtils'
describe('fileTypeUtils', () => {
@ -67,35 +66,23 @@ describe('fileTypeUtils', () => {
})
})
describe('mediaFileUrlToEmbeddedIframeUrl', () => {
it('creates iframe URL for audio', () => {
const fileurl = 'http://host:port/path/to/file?query=1'
const url = mediaFileUrlToEmbeddedIframeUrl(fileurl, 'audio')
expect(url).toBe(`/media_objects_iframe/?type=audio&mediahref=${encodeURIComponent(fileurl)}`)
})
it('creates iframe URL for video', () => {
const fileurl = 'http://host:port/path/to/file?query=1'
const url = mediaFileUrlToEmbeddedIframeUrl(fileurl, 'video')
expect(url).toBe(`/media_objects_iframe/?type=video&mediahref=${encodeURIComponent(fileurl)}`)
})
})
describe('embedded_iframe_url_fromFile', () => {
it("returns url from input file's embedded_iframe_url", () => {
describe('mediaPlayerURLFromFile', () => {
it("creates url from input file's embedded_iframe_url", () => {
const file = {
embedded_iframe_url: '/media_objects_iframe/m-media_object_id'
embedded_iframe_url: '/media_objects_iframe/m-media_object_id',
type: 'video/mov'
}
const url = embedded_iframe_url_fromFile(file)
expect(url).toBe(file.embedded_iframe_url)
const url = mediaPlayerURLFromFile(file)
expect(url).toBe('/media_objects_iframe/m-media_object_id?type=video')
})
it("creates url from file's media_entry_id", () => {
const file = {
media_entry_id: 'm-media_id'
media_entry_id: 'm-media_id',
content_type: 'audio/mp3'
}
const url = embedded_iframe_url_fromFile(file)
expect(url).toBe('/media_objects_iframe/m-media_id')
const url = mediaPlayerURLFromFile(file)
expect(url).toBe('/media_objects_iframe/m-media_id?type=audio')
})
it("creates url from file's url", () => {
@ -103,10 +90,8 @@ describe('fileTypeUtils', () => {
'content-type': 'video/mov',
url: 'http://origin/path/to/file'
}
const url = embedded_iframe_url_fromFile(file)
expect(url).toBe(
`/media_objects_iframe/?type=video&mediahref=${encodeURIComponent(file.url)}`
)
const url = mediaPlayerURLFromFile(file)
expect(url).toBe('/media_objects_iframe?mediahref=/path/to/file&type=video')
})
it("returns undefined if the file isn't media", () => {
@ -114,8 +99,17 @@ describe('fileTypeUtils', () => {
'content-type': 'text/palin',
url: 'http://origin/path/to/file'
}
const url = embedded_iframe_url_fromFile(file)
const url = mediaPlayerURLFromFile(file)
expect(url).toBe(undefined)
})
it("includes the file verifier if it's part of the file's url", () => {
const file = {
'content-type': 'video/mov',
url: 'http://origin/path/to/file?verifier=xyzzy'
}
const url = mediaPlayerURLFromFile(file)
expect(url).toBe('/media_objects_iframe?mediahref=/path/to/file&verifier=xyzzy&type=video')
})
})
})

View File

@ -16,7 +16,7 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import {downloadToWrap} from '../../../common/fileUrl'
import {parse} from 'url'
import {
IconDocumentLine,
IconMsExcelLine,
@ -70,24 +70,23 @@ export function isText(type) {
return /^text/.test(type)
}
export function mediaFileUrlToEmbeddedIframeUrl(media_url, content_type) {
export function mediaPlayerURLFromFile(file) {
// why oh why aren't we consistent?
const content_type = file['content-type'] || file.content_type || file.type
const type = content_type.replace(/\/.*$/, '')
return `/media_objects_iframe/?type=${type}&mediahref=${encodeURIComponent(media_url)}`
}
export function embedded_iframe_url_fromFile(file) {
if (file.embedded_iframe_url) {
return file.embedded_iframe_url
return `${file.embedded_iframe_url}?type=${type}`
}
if (file.media_entry_id && file.media_entry_id !== 'maybe') {
return `/media_objects_iframe/${file.media_entry_id}`
}
const content_type = file['content-type']
if (isAudioOrVideo(content_type)) {
return mediaFileUrlToEmbeddedIframeUrl(downloadToWrap(file.url), content_type)
}
if (file.media_entry_id && file.media_entry_id !== 'maybe') {
return `/media_objects_iframe/${file.media_entry_id}?type=${type}`
}
const parsed_url = parse(file.url || file.href, true)
const verifier = parsed_url.query.verifier ? `&verifier=${parsed_url.query.verifier}` : ''
return `/media_objects_iframe?mediahref=${parsed_url.pathname}${verifier}&type=${type}`
}
return undefined
}

View File

@ -157,8 +157,10 @@ export function embedUploadResult(results, selectedTabType) {
?.mceInstance()
?.selection.collapse()
// when we record audio, notorious thinks it's a video. use the content type we got
// from the recoreded file, not the returned media object.
bridge.embedMedia({
id: embedData.id,
id: results.id,
embedded_iframe_url: results.embedded_iframe_url,
href: results.href || results.url,
media_id: results.media_id,

View File

@ -27,7 +27,7 @@ describe('fileEmbed', () => {
}
it('defaults to file', () => {
assert.equal(fileEmbed({}).type, 'file')
assert.strictEqual(fileEmbed({}).type, 'file')
})
it('uses content-type to identify video and audio', () => {
@ -38,62 +38,45 @@ describe('fileEmbed', () => {
)
const notvideo = fileEmbed(getBaseFile({'content-type': 'x-video/mp4', preview_url: undefined}))
assert.equal(video.type, 'video')
assert.equal(video.id, 'maybe')
assert.equal(audio.type, 'audio')
assert.equal(audio.id, 'maybe')
assert.equal(notaudio.type, 'file')
assert.equal(notvideo.type, 'file')
})
it('returns media entry id if provided', () => {
const video = fileEmbed(
getBaseFile({
'content-type': 'video/mp4',
media_entry_id: '42'
})
)
assert.equal(video.id, '42')
})
it('returns maybe in place of media entry id if not provided', () => {
const video = fileEmbed(getBaseFile({'content-type': 'video/mp4'}))
assert.equal(video.id, 'maybe')
assert.strictEqual(video.type, 'video')
assert.strictEqual(audio.type, 'audio')
assert.strictEqual(notaudio.type, 'file')
assert.strictEqual(notvideo.type, 'file')
})
it('picks scribd if there is a preview_url', () => {
const scribd = fileEmbed(getBaseFile({preview_url: 'some-url'}))
assert.equal(scribd.type, 'scribd')
assert.strictEqual(scribd.type, 'scribd')
})
it('uses content-type to identify images', () => {
const png = fileEmbed(getBaseFile({'content-type': 'image/png'}))
const svg = fileEmbed(getBaseFile({'content-type': 'image/svg+xml'}))
assert.equal(png.type, 'image')
assert.equal(svg.type, 'image')
assert.strictEqual(png.type, 'image')
assert.strictEqual(svg.type, 'image')
})
})
describe('mimeClass', () => {
it('returns mime_class attribute if present', () => {
const mime_class = 'wooper'
assert.equal(mimeClass({mime_class}), mime_class)
assert.strictEqual(mimeClass({mime_class}), mime_class)
})
it('returns value corresponding to provided `content-type`', () => {
assert.equal(mimeClass({'content-type': 'video/mp4'}), 'video')
assert.equal(mimeClass({'content-type': 'audio/webm'}), 'audio')
assert.equal(mimeClass({'content-type': 'image/svg+xml'}), 'image')
assert.equal(mimeClass({'content-type': 'image/webp'}), 'file')
assert.equal(mimeClass({'content-type': 'application/vnd.ms-powerpoint'}), 'ppt')
assert.strictEqual(mimeClass({'content-type': 'video/mp4'}), 'video')
assert.strictEqual(mimeClass({'content-type': 'audio/webm'}), 'audio')
assert.strictEqual(mimeClass({'content-type': 'image/svg+xml'}), 'image')
assert.strictEqual(mimeClass({'content-type': 'image/webp'}), 'file')
assert.strictEqual(mimeClass({'content-type': 'application/vnd.ms-powerpoint'}), 'ppt')
})
it('returns value corresponding to provided `type`', () => {
assert.equal(mimeClass({type: 'video/mp4'}), 'video')
assert.equal(mimeClass({type: 'audio/webm'}), 'audio')
assert.equal(mimeClass({type: 'image/svg+xml'}), 'image')
assert.equal(mimeClass({type: 'image/webp'}), 'file')
assert.equal(mimeClass({type: 'application/vnd.ms-powerpoint'}), 'ppt')
assert.strictEqual(mimeClass({type: 'video/mp4'}), 'video')
assert.strictEqual(mimeClass({type: 'audio/webm'}), 'audio')
assert.strictEqual(mimeClass({type: 'image/svg+xml'}), 'image')
assert.strictEqual(mimeClass({type: 'image/webp'}), 'file')
assert.strictEqual(mimeClass({type: 'application/vnd.ms-powerpoint'}), 'ppt')
})
})