From 64bfc9ae75bb4086de20cdf7b34127e54ee238bf Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Mon, 24 Jan 2022 15:07:59 -0500 Subject: [PATCH 1/3] fix resource_link regex, make non-greedy Fixes https://github.com/mitodl/ocw-studio/issues/917 and https://github.com/mitodl/ocw-studio/issues/903. Before the regex was greedy so it would not stop at the end of the first resource_link. Two resource links on the same lin would be gobbled up as one. The new regex is still pretty simple but will be fooled by resource_link titles that include literal `" >}}` values. This is a pretty edgy edge case. We could get around this in the future by escpaing quotations in the link titles and using a different regex (or parsing some other way). But right now the unescaped quotation characters are not causing any problems other than this, so lets keep this fix simple. --- .../ResourceLinkMarkdownSyntax.test.ts | 21 ++++++++++++++++++- .../plugins/ResourceLinkMarkdownSyntax.ts | 13 ++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.test.ts b/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.test.ts index 1147f4358..10b0daeb2 100644 --- a/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.test.ts +++ b/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.test.ts @@ -1,7 +1,7 @@ jest.mock("@ckeditor/ckeditor5-utils/src/version") import ResourceLink from "@mitodl/ckeditor5-resource-link/src/link" -import Markdown from "./Markdown" +import Markdown, { MarkdownDataProcessor } from "./Markdown" import { createTestEditor, markdownTest } from "./test_util" import { turndownService } from "../turndown" @@ -46,4 +46,23 @@ describe("ResourceLink plugin", () => { '

text here

' ) }) + + it("should serialize multiple links to and from markdown", async () => { + const editor = await getEditor("") + markdownTest( + editor, + 'dogs {{< resource_link uuid1 "woof" >}} cats {{< resource_link uuid2 "meow" >}}, cool', + '

dogs woof cats meow, cool

' + ) + }) + + it("[BUG] does not behave well if link title ends in backslash", async () => { + const editor = await getEditor("") + const { md2html } = (editor.data + .processor as unknown) as MarkdownDataProcessor + expect(md2html('{{< resource_link uuid123 "bad \\" >}}')).toBe( + // This is wrong. Should not end in </a> + '

bad </a>

' + ) + }) }) diff --git a/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts b/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts index 07e026d85..3b5bc612b 100644 --- a/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts +++ b/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts @@ -5,8 +5,6 @@ import { editor } from "@ckeditor/ckeditor5-core" import MarkdownSyntaxPlugin from "./MarkdownSyntaxPlugin" import { TurndownRule } from "../../../types/ckeditor_markdown" -export const RESOURCE_LINK_SHORTCODE_REGEX = /{{< resource_link (\S+) "(.+)" >}}/g - import { RESOURCE_LINK_CKEDITOR_CLASS, RESOURCE_LINK @@ -27,6 +25,17 @@ import { * The ResourceEmbed plugin itself is provided via our fork of CKEditor's * 'link' plugin. */ + +export const RESOURCE_LINK_SHORTCODE_REGEX = /{{< resource_link (\S+) "(.*?)" >}}/g +/** + * (\S+) to match and capture the UUID + * "(.*?)" to match and capture the label text + * + * Limitations: + * - gets fooled by label texts that include literal `" >}}` values. For + * example, < resource_link uuid123 "silly " >}} link" >}}. + */ + export default class ResourceLinkMarkdownSyntax extends MarkdownSyntaxPlugin { constructor(editor: editor.Editor) { super(editor) From f22526a7fe69680dab30d0bcbf852aadbfbce1b2 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Mon, 24 Jan 2022 16:33:04 -0500 Subject: [PATCH 2/3] move regex back up --- .../plugins/ResourceLinkMarkdownSyntax.ts | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts b/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts index 3b5bc612b..ad32956d3 100644 --- a/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts +++ b/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts @@ -10,6 +10,16 @@ import { RESOURCE_LINK } from "@mitodl/ckeditor5-resource-link/src/constants" +export const RESOURCE_LINK_SHORTCODE_REGEX = /{{< resource_link (\S+) "(.*?)" >}}/g +/** + * (\S+) to match and capture the UUID + * "(.*?)" to match and capture the label text + * + * Limitations: + * - gets fooled by label texts that include literal `" >}}` values. For + * example, < resource_link uuid123 "silly " >}} link" >}}. + */ + /** * Class for defining Markdown conversion rules for Resource links * @@ -25,17 +35,6 @@ import { * The ResourceEmbed plugin itself is provided via our fork of CKEditor's * 'link' plugin. */ - -export const RESOURCE_LINK_SHORTCODE_REGEX = /{{< resource_link (\S+) "(.*?)" >}}/g -/** - * (\S+) to match and capture the UUID - * "(.*?)" to match and capture the label text - * - * Limitations: - * - gets fooled by label texts that include literal `" >}}` values. For - * example, < resource_link uuid123 "silly " >}} link" >}}. - */ - export default class ResourceLinkMarkdownSyntax extends MarkdownSyntaxPlugin { constructor(editor: editor.Editor) { super(editor) From 81868bc93eb06e1cd166b2eae7c76f0861e84233 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Mon, 24 Jan 2022 16:47:47 -0500 Subject: [PATCH 3/3] move comment above declaration --- static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts b/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts index ad32956d3..f0378228c 100644 --- a/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts +++ b/static/js/lib/ckeditor/plugins/ResourceLinkMarkdownSyntax.ts @@ -10,7 +10,6 @@ import { RESOURCE_LINK } from "@mitodl/ckeditor5-resource-link/src/constants" -export const RESOURCE_LINK_SHORTCODE_REGEX = /{{< resource_link (\S+) "(.*?)" >}}/g /** * (\S+) to match and capture the UUID * "(.*?)" to match and capture the label text @@ -19,6 +18,7 @@ export const RESOURCE_LINK_SHORTCODE_REGEX = /{{< resource_link (\S+) "(.*?)" >} * - gets fooled by label texts that include literal `" >}}` values. For * example, < resource_link uuid123 "silly " >}} link" >}}. */ +export const RESOURCE_LINK_SHORTCODE_REGEX = /{{< resource_link (\S+) "(.*?)" >}}/g /** * Class for defining Markdown conversion rules for Resource links