From 7d1e0399efb52744aab8b3bfbb20d0255e655c82 Mon Sep 17 00:00:00 2001 From: Davis Hyer Date: Wed, 15 Feb 2023 01:55:25 +0000 Subject: [PATCH] Revert "Clean up RCE content before loading editor" This reverts commit f7d97c3c5b036c090bf8439463a4524b00ce8bea. Reason for revert: MAT-1226 Change-Id: I9dc768b8c268cc68f3ee037277295ff186efb639 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/310318 Tested-by: Service Cloud Jenkins Reviewed-by: Stephen Kacsmark QA-Review: Jeremy Stanley Product-Review: Stephen Kacsmark --- .../instructure_helper.js | 10 +- packages/canvas-rce/src/rce/RCEWrapper.js | 31 ++--- .../rce/__tests__/transformContent.test.ts | 96 --------------- .../canvas-rce/src/rce/transformContent.ts | 81 ------------- .../canvas-rce/src/util/__tests__/url.test.ts | 110 ------------------ packages/canvas-rce/src/util/url-util.ts | 78 ------------- 6 files changed, 18 insertions(+), 388 deletions(-) delete mode 100644 packages/canvas-rce/src/rce/__tests__/transformContent.test.ts delete mode 100644 packages/canvas-rce/src/rce/transformContent.ts delete mode 100644 packages/canvas-rce/src/util/__tests__/url.test.ts delete mode 100644 packages/canvas-rce/src/util/url-util.ts diff --git a/packages/canvas-rce/src/enhance-user-content/instructure_helper.js b/packages/canvas-rce/src/enhance-user-content/instructure_helper.js index 3c944c822658..05d3f8441d21 100644 --- a/packages/canvas-rce/src/enhance-user-content/instructure_helper.js +++ b/packages/canvas-rce/src/enhance-user-content/instructure_helper.js @@ -19,9 +19,8 @@ import htmlEscape from 'escape-html' import formatMessage from '../format-message' import {showFlashAlert} from '../common/FlashAlert' -import {isPreviewable, loadDocPreview, removeLoadingImage, showLoadingImage} from './doc_previews' +import {isPreviewable, loadDocPreview, showLoadingImage, removeLoadingImage} from './doc_previews' import {show} from './jqueryish_funcs' -import {parseUrlOrNull} from '../util/url-util' const youTubeRegEx = /^https?:\/\/(www\.youtube\.com\/watch.*v(=|\/)|youtu\.be\/)([^&#]*)/ export function youTubeID(path) { @@ -150,7 +149,12 @@ export function showFilePreviewInline(event, canvasOrigin, disableGooglePreviews $link.setAttribute('aria-expanded', 'true') if (canvasOrigin && canvadoc_session_url !== null) { - canvadoc_session_url = parseUrlOrNull(canvadoc_session_url, canvasOrigin)?.toString() + try { + canvadoc_session_url = (new URL(canvadoc_session_url, canvasOrigin)).toString(); + } + catch(_ex){ + canvadoc_session_url = null + } } const $div = document.querySelector(`[id="${$link.getAttribute('aria-controls')}"]`) diff --git a/packages/canvas-rce/src/rce/RCEWrapper.js b/packages/canvas-rce/src/rce/RCEWrapper.js index 022df9bc47da..ba43df2b2ba5 100644 --- a/packages/canvas-rce/src/rce/RCEWrapper.js +++ b/packages/canvas-rce/src/rce/RCEWrapper.js @@ -40,11 +40,11 @@ import {sanitizePlugins} from './sanitizePlugins' import RCEGlobals from './RCEGlobals' import defaultTinymceConfig from '../defaultTinymceConfig' import { - FS_CHANGEEVENT, - FS_ELEMENT, FS_ENABLED, - FS_EXIT, + FS_ELEMENT, FS_REQUEST, + FS_EXIT, + FS_CHANGEEVENT, instuiPopupMountNode, } from '../util/fullscreenHelpers' @@ -70,7 +70,6 @@ import launchWordcountModal from './plugins/instructure_wordcount/clickCallback' import styles from '../skins/skin-delta.css' import skinCSSBinding from 'tinymce/skins/ui/oxide/skin.min.css' import contentCSSBinding from 'tinymce/skins/ui/oxide/content.css' -import {transformRceContentForEditing} from './transformContent' const RestoreAutoSaveModal = React.lazy(() => import('./RestoreAutoSaveModal')) const RceHtmlEditor = React.lazy(() => import('./RceHtmlEditor')) @@ -315,11 +314,6 @@ class RCEWrapper extends React.Component { this._prettyHtmlEditorRef = React.createRef() this._showOnFocusButton = null - // Processed initial content - this.initialContent = transformRceContentForEditing(this.props.defaultContent, { - origin: this.props.canvasOrigin || window?.location?.origin, - }) - injectTinySkin() // FWIW, for historic reaasons, the height does not include the @@ -905,21 +899,18 @@ class RCEWrapper extends React.Component { if (this.mceInstance().isDirty()) { return true } - const currentHtml = this.isHidden() ? this.textareaValue() : this.mceInstance()?.getContent() - return currentHtml !== this._mceSerializedInitialHtml + const content = this.isHidden() ? this.textareaValue() : this.mceInstance()?.getContent() + return content !== this.cleanInitialContent() } - /** - * Holds a copy of the initial content of the editor as serialized by tinyMCE to normalize it. - */ - get _mceSerializedInitialHtml() { - if (!this._mceSerializedInitialHtmlCached) { + cleanInitialContent() { + if (!this._cleanInitialContent) { const el = window.document.createElement('div') - el.innerHTML = this.initialContent + el.innerHTML = this.props.defaultContent const serializer = this.mceInstance().serializer - this._mceSerializedInitialHtmlCached = serializer.serialize(el, {getInner: true}) + this._cleanInitialContent = serializer.serialize(el, {getInner: true}) } - return this._mceSerializedInitialHtmlCached + return this._cleanInitialContent } isHtmlView() { @@ -2025,7 +2016,7 @@ class RCEWrapper extends React.Component { id={mceProps.textareaId} textareaName={mceProps.name} init={this.tinymceInitOptions} - initialValue={this.initialContent} + initialValue={mceProps.defaultContent} onInit={this.onInit} onClick={this.handleFocusEditor} onKeypress={this.handleFocusEditor} diff --git a/packages/canvas-rce/src/rce/__tests__/transformContent.test.ts b/packages/canvas-rce/src/rce/__tests__/transformContent.test.ts deleted file mode 100644 index b05d9616c2d7..000000000000 --- a/packages/canvas-rce/src/rce/__tests__/transformContent.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -/* - * Copyright (C) 2022 - present Instructure, Inc. - * - * This file is part of Canvas. - * - * Canvas is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the Free - * Software Foundation, version 3 of the License. - * - * Canvas is distributed in the hope that it will be useful, but WITHOUT ANY - * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR - * A PARTICULAR PURPOSE. See the GNU Affero General Public License for more - * details. - * - * You should have received a copy of the GNU Affero General Public License along - * with this program. If not, see . - */ - -import { - attributeNamesToRemove, - transformRceContentForEditing, - TransformRceContentForEditingOptions, -} from '../transformContent' - -describe('transformRceContentForEditing', () => { - const defaultOptions: TransformRceContentForEditingOptions = { - origin: 'http://canvas.com', - } - - it('should not modify falsey inputs', () => { - expect(transformRceContentForEditing(null, defaultOptions)).toEqual(null) - expect(transformRceContentForEditing(undefined, defaultOptions)).toEqual(undefined) - expect(transformRceContentForEditing('', defaultOptions)).toEqual('') - }) - - it('should relativize urls', () => { - expect( - transformRceContentForEditing( - '' + - '' + - '' + - '
' + - '' + - '' + - '
', - defaultOptions - ) - ).toEqual( - '' + - '' + - '' + - '
' + - '' + - '' + - '
' - ) - }) - - it('should remove unnecessary attributes', () => { - const elements = [ - {value: 'iframe', shouldTransform: true}, - {value: 'img', shouldTransform: true, selfClosing: true}, - {value: 'embed', shouldTransform: true, selfClosing: true}, - ] - - const attributes = [ - ...attributeNamesToRemove.map(value => ({ - value, - shouldRemove: true, - })), - {value: 'random', shouldRemove: false}, - ] - - elements.forEach(element => { - attributes.forEach(attribute => { - const elementWithAttribute = element.selfClosing - ? `<${element.value} ${attribute.value}="whatever">` - : `<${element.value} ${attribute.value}="whatever">` - const withAttributeHtml = `${elementWithAttribute}
${elementWithAttribute}
` - - const elementWithoutAttribute = element.selfClosing - ? `<${element.value}>` - : `<${element.value}>` - const withoutAttributeHtml = `${elementWithoutAttribute}
${elementWithoutAttribute}
` - - const transformedHtml = transformRceContentForEditing(withAttributeHtml, defaultOptions) - - if (attribute.shouldRemove) { - expect(transformedHtml).toEqual(withoutAttributeHtml) - } else { - expect(transformedHtml).toEqual(withAttributeHtml) - } - }) - }) - }) -}) diff --git a/packages/canvas-rce/src/rce/transformContent.ts b/packages/canvas-rce/src/rce/transformContent.ts deleted file mode 100644 index 4ee516cb6eaa..000000000000 --- a/packages/canvas-rce/src/rce/transformContent.ts +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright (C) 2022 - present Instructure, Inc. - * - * This file is part of Canvas. - * - * Canvas is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the Free - * Software Foundation, version 3 of the License. - * - * Canvas is distributed in the hope that it will be useful, but WITHOUT ANY - * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR - * A PARTICULAR PURPOSE. See the GNU Affero General Public License for more - * details. - * - * You should have received a copy of the GNU Affero General Public License along - * with this program. If not, see . - */ - -import {relativeHttpUrlForHostname} from '../util/url-util' - -export const attributeNamesToUrlRelativize = ['href', 'cite', 'src', 'data'] -export const attributeNamesToRemove = ['data-api-endpoint', 'data-api-returntype'] - -/** - * Transforms a block of HTML for use within the Rich Content Editor, normalizing content to remove extraneous - * things added by the server. - * - * @param inputHtml - * @param options - */ -export function transformRceContentForEditing( - inputHtml: string | null | undefined, - options: TransformRceContentForEditingOptions -) { - if (!inputHtml) { - // It's important to return null/undefined here if that was passed in, otherwise tests fail because - // the change-detection logic doesn't work correctly. - return inputHtml - } - - let container: HTMLElement | null - - try { - container = new DOMParser().parseFromString(inputHtml, 'text/html').querySelector('body') - } catch (e) { - return inputHtml - } - - if (!container) { - return inputHtml - } - - // Relativize URLs in attribute - for (const attributeName of attributeNamesToUrlRelativize) { - container.querySelectorAll(`[${attributeName}]`).forEach(element => { - const attributeValue = element.getAttribute(attributeName) - - if (attributeValue) { - element.setAttribute( - attributeName, - relativeHttpUrlForHostname(attributeValue, options.origin) - ) - } - }) - } - - // Remove extraneous attributes - container - .querySelectorAll(attributeNamesToRemove.map(it => `[${it}]`).join(',')) - .forEach(element => { - for (const attributeName of attributeNamesToRemove) { - element.removeAttribute(attributeName) - } - }) - - return container.innerHTML -} - -export interface TransformRceContentForEditingOptions { - origin: string -} diff --git a/packages/canvas-rce/src/util/__tests__/url.test.ts b/packages/canvas-rce/src/util/__tests__/url.test.ts deleted file mode 100644 index 6cb1452f9adb..000000000000 --- a/packages/canvas-rce/src/util/__tests__/url.test.ts +++ /dev/null @@ -1,110 +0,0 @@ -/* - * Copyright (C) 2022 - present Instructure, Inc. - * - * This file is part of Canvas. - * - * Canvas is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the Free - * Software Foundation, version 3 of the License. - * - * Canvas is distributed in the hope that it will be useful, but WITHOUT ANY - * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR - * A PARTICULAR PURPOSE. See the GNU Affero General Public License for more - * details. - * - * You should have received a copy of the GNU Affero General Public License along - * with this program. If not, see . - */ - -import {parseUrlOrNull, relativeHttpUrlForHostname} from '../url-util' - -describe('parseUrlOrNull', () => { - it('should parse a valid url', () => { - expect(parseUrlOrNull('https://foobar.local/123')?.toString()).toEqual( - 'https://foobar.local/123' - ) - }) - - it('should parse a valid url with an origin', () => { - expect(parseUrlOrNull('/123', 'https://foobar.local')?.toString()).toEqual( - 'https://foobar.local/123' - ) - }) - - it('should handle falsey values', () => { - expect(parseUrlOrNull(null)).toEqual(null) - }) - - it('should handle invalid URLs', () => { - expect(parseUrlOrNull('!@#!@#')).toEqual(null) - }) -}) - -describe('relativeHttpUrlForHostname', () => { - it('should only relativize urls when appropriate', () => { - const canvasOrigins = [ - {value: 'HTTP://CANVAS.COM', shouldTransform: true}, - {value: 'HTTPS://CANVAS.COM', shouldTransform: true}, - {value: 'http://canvas.com', shouldTransform: true}, - {value: 'https://canvas.com', shouldTransform: true}, - {value: 'http://canvas.com:80', shouldTransform: true}, - {value: 'https://canvas.com:443', shouldTransform: true}, - {value: 'http://canvas.com:443', shouldTransform: true}, - {value: 'https://canvas.com:80', shouldTransform: true}, - {value: 'http://canvas.com:1234', shouldTransform: true}, - ] - - const urlOrigins = [ - {value: 'HTTP://CANVAS.COM', shouldTransform: true}, - {value: 'HTTPS://CANVAS.COM', shouldTransform: true}, - - {value: 'http://canvas.com', shouldTransform: true}, - {value: 'https://canvas.com', shouldTransform: true}, - {value: 'ftp://canvas.com', shouldTransform: false}, - - {value: 'http://canvas.com:80', shouldTransform: true}, - {value: 'https://canvas.com:443', shouldTransform: true}, - {value: 'http://canvas.com:443', shouldTransform: true}, - {value: 'https://canvas.com:80', shouldTransform: true}, - {value: 'http://canvas.com:1234', shouldTransform: true}, - {value: 'https://canvas.com:1234', shouldTransform: true}, - - {value: 'http://other.canvas.com', shouldTransform: false}, - {value: 'https://other.canvas.com', shouldTransform: false}, - {value: 'https://google.com', shouldTransform: false}, - {value: 'http://nowhere.com', shouldTransform: false}, - ] - - const paths = [ - {value: '/other-page', shouldTransform: true}, - {value: '/avocado.jpg', shouldTransform: true}, - {value: '!@#$%^', shouldTransform: false}, - ] - - const elements = [ - {value: 'iframe', shouldTransform: true}, - {value: 'img', shouldTransform: true, selfClosing: true}, - {value: 'embed', shouldTransform: true, selfClosing: true}, - ] - - canvasOrigins.forEach(canvasOrigin => { - urlOrigins.forEach(urlOrigin => { - paths.forEach(path => { - elements.forEach(element => { - const shouldTransform = [canvasOrigin, urlOrigin, path, element].every( - it => it.shouldTransform - ) - - const absoluteUrl = `${urlOrigin.value}${path.value}` - const relativeUrl = path.value - - const transformedUrl = relativeHttpUrlForHostname(absoluteUrl, canvasOrigin.value) - const expectedUrl = shouldTransform ? relativeUrl : absoluteUrl - - expect(transformedUrl).toEqual(expectedUrl) - }) - }) - }) - }) - }) -}) diff --git a/packages/canvas-rce/src/util/url-util.ts b/packages/canvas-rce/src/util/url-util.ts deleted file mode 100644 index ae4449c30069..000000000000 --- a/packages/canvas-rce/src/util/url-util.ts +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright (C) 2023 - present Instructure, Inc. - * - * This file is part of Canvas. - * - * Canvas is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the Free - * Software Foundation, version 3 of the License. - * - * Canvas is distributed in the hope that it will be useful, but WITHOUT ANY - * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR - * A PARTICULAR PURPOSE. See the GNU Affero General Public License for more - * details. - * - * You should have received a copy of the GNU Affero General Public License along - * with this program. If not, see . - */ - -/** - * Attempts to build a URL from the given string, and returns null if it is not a valid URL, rather than - * throwing an exception, as the URL constructor does. - */ -export function parseUrlOrNull(url: string | null | undefined, base?: string | URL): URL | null { - if (!url) return null - - try { - return new URL(url, base) - } catch (e) { - return null - } -} - -/** - * Converts the given URL into a relative URL if it meets the following criteria: - * - is parsable by the browser URL class - * - has the HTTP or HTTPS protocol - * - has the same hostname as the given origin - * - * Note: This will relativize URLs where the ports don't match. This is intentional, as ports really shouldn't - * matter for RCE HTTP content, and it can solve issues where an extraneous port is added (e.g. :80 on an http url) - * or when running locally and the port is different. There isn't a security issue because the user could just manually - * put in the transformed content anyways. - * - * @param inputValue URL to relativize - * @param origin Origin to check for - */ -export function relativeHttpUrlForHostname( - inputValue: TInput, - origin: string -): TInput { - if (!inputValue) { - return inputValue - } - - if (!inputValue?.match(/^https?:/i)) { - // Already relative or not a http/https protocol url - return inputValue - } - - const url = parseUrlOrNull(inputValue) - if (!url) { - return inputValue - } - - // Handle the simple case of origins matching - if (url.origin === origin.toLowerCase()) { - return url.pathname as TInput - } - - // Handle the more complex case of hostname/port matching - const originHostname = parseUrlOrNull(origin)?.hostname - - if (url.hostname.toLowerCase() === originHostname?.toLowerCase()) { - return url.pathname as TInput - } else { - return inputValue - } -}