FIX: BBCode tag parser

Wasn't quite handling the cases where a closing bracket `]` was used in the value of one of the attributes.

```markdown
[chat quote=user channel="[broken]"]
```

Would not be correctly parsed because we would _greedily_ use the first `]` as the end of the tag even though it might be a valid character when inside proper quotes.

c39a4de139/app/assets/javascripts/discourse-markdown-it/src/features/bbcode-block.js (L62)

Re-wrote the `parseBBCodeTag` to properly handle the following cases

- A closing tag (aka `[/name]`) which are easy since they don't have any attributes
- An old `[quote=...]` format we used that doesn't uses quotes but still has various attributes of the form `key:value`
- All three valid BBCode opening tag formats we support
  - `[name]` without any attributes
  - `[name=foo]` with a default value
  - `[name foo=bar]` with some attributes

Ended up having to fix/rewrite the few bbcode rules that were using the `parseBBCodeTag` function, namely `d-wrap` and `discourse-local-dates`.

While working on this, I think I also found a way to get rid the of shims we had in place so that plugins could use the `parseBBCodeTag` function.

Reference - https://meta.discourse.org/t/having-a-right-bracket-in-a-channel-name-breaks-all-quotes-from-that-channel/308439

app/assets/javascripts/discourse-markdown-it/src/features/bbcode-block.js

@@ -1,4 +1,24 @@
-let isWhiteSpace;
+let isWhiteSpace, escapeHtml;
+
+function camelCaseToDash(str) {
+  return str.replace(/([a-zA-Z])(?=[A-Z])/g, "$1-").toLowerCase();
+}
+
+export function applyDataAttributes(token, attributes, defaultName) {
+  const { _default, ...attrs } = attributes;
+
+  if (_default && defaultName) {
+    attrs[defaultName] = _default;
+  }
+
+  for (let key of Object.keys(attrs).sort()) {
+    const value = escapeHtml(attrs[key]);
+    key = camelCaseToDash(key.replace(/[^a-z0-9-]/gi, ""));
+    if (value && key && key.length > 1) {
+      token.attrSet(`data-${key}`, value);
+    }
+  }
+}
 
 function trailingSpaceOnly(src, start, max) {
   for (let i = start; i < max; i++) {
@@ -14,89 +34,121 @@ function trailingSpaceOnly(src, start, max) {
   return true;
 }
 
-const ATTR_REGEX =
+// Easiest case is the closing tag which never has any attributes
-  /^\s*=(.+)$|((([a-z0-9]*)\s*)=)([“”"][^“”"]*[“”"]|['][^']*[']|[^"'“”]\S*)/gi;
+const BBCODE_CLOSING_TAG_REGEXP = /^\[\/([-\w]+)\]/i;
+
+// Old case where we supported attributes without quotation marks
+const BBCODE_QUOTE_TAG_REGEXP = /^\[quote=([-\w,: ]+)\]/i;
+
+// Most common quotation marks.
+// More can be found at https://en.wikipedia.org/wiki/Quotation_mark
+const QUOTATION_MARKS = [`""`, `''`, `“”`, `‘’`, `„“`, `‚’`, `«»`, `‹›`];
+
+const QUOTATION_MARKS_NO_MATCH = QUOTATION_MARKS.map(
+  ([a, b]) => `${a}[^${b}]+${b}`
+).join("|");
+
+const QUOTATION_MARKS_WITH_MATCH = QUOTATION_MARKS.map(
+  ([a, b]) => `${a}([^${b}]+)${b}`
+).join("|");
+
+// This is used to match a **valid** opening tag
+// NOTE: it does not match the closing bracket "]" because it makes the regexp too slow
+// due to the backtracking. So we check for the "]" manually.
+const BBCODE_TAG_REGEXP = new RegExp(
+  `\\[(?:(?:[-\\w]+(?:=(?:${QUOTATION_MARKS_NO_MATCH}|[^\\s\\]]+))?)+\\s*)+`,
+  "i"
+);
+
+// This is used to parse attributes of the form key=value
+// Where value might have some quotation marks
+const BBCODE_ATTR_REGEXP = new RegExp(
+  `([-\\w]+)(?:=(?:${QUOTATION_MARKS_WITH_MATCH}|([^\\s\\]]+)))?`,
+  "gi"
+);
 
-// parse a tag [test a=1 b=2] to a data structure
-// {tag: "test", attrs={a: "1", b: "2"}
 export function parseBBCodeTag(src, start, max, multiline) {
-  let i;
+  let m;
-  let tag;
+  const text = src.slice(start, max);
-  let attrs = {};
-  let closed = false;
-  let length = 0;
-  let closingTag = false;
 
-  // closing tag
+  // CASE 1 - closing tag
-  if (src.charCodeAt(start + 1) === 47) {
+  m = BBCODE_CLOSING_TAG_REGEXP.exec(text);
-    closingTag = true;
+
-    start += 1;
+  if (m && m[0] && m[1]) {
+    if (multiline && !trailingSpaceOnly(src, start + m[0].length, max)) {
+      return null;
     }
 
-  for (i = start + 1; i < max; i++) {
+    return {
-    if (!/[a-z]/i.test(src[i])) {
+      tag: m[1].toLowerCase(),
-      break;
+      closing: true,
-    }
+      length: m[0].length,
+    };
   }
 
-  tag = src.slice(start + 1, i);
+  // CASE 2 - [quote=...] tag (without quotes)
+  m = BBCODE_QUOTE_TAG_REGEXP.exec(text);
 
-  if (!tag) {
+  if (m && m[0] && m[1]) {
-    return;
+    if (multiline && !trailingSpaceOnly(src, start + m[0].length, max)) {
+      return null;
     }
 
-  if (closingTag) {
+    return {
-    if (src[i] === "]") {
+      tag: "quote",
-      if (multiline && !trailingSpaceOnly(src, i + 1, max)) {
+      length: m[0].length,
-        return;
+      attrs: { _default: m[1] },
+    };
   }
 
-      tag = tag.toLowerCase();
+  // CASE 3 - regular opening tag
+  m = BBCODE_TAG_REGEXP.exec(text);
+  const bbcode = m ? m[0] : null;
 
-      return { tag, length: tag.length + 3, closing: true };
+  if (!bbcode) {
-    }
+    return null;
-    return;
   }
 
-  for (; i < max; i++) {
+  if (text.length <= bbcode.length || text[bbcode.length] !== "]") {
-    if (src[i] === "]") {
+    return null;
-      closed = true;
-      break;
-    }
   }
 
-  if (closed) {
+  const r = {};
-    length = i - start + 1;
 
-    let raw = src.slice(start + tag.length + 1, i);
+  while ((m = BBCODE_ATTR_REGEXP.exec(bbcode))) {
+    const [, key, ...v] = m;
+    const value = v.find(Boolean);
 
-    // trivial parser that is going to have to be rewritten at some point
+    if (!key) {
-    if (raw) {
+      return null;
-      let match, key, val;
+    }
 
-      while ((match = ATTR_REGEX.exec(raw))) {
+    if (!r.tag) {
-        if (match[1]) {
+      r.tag = key.toLowerCase();
-          key = "_default";
+      r.length = bbcode.length + 1;
+      if (m.index === 1) {
+        r.attrs = {};
+        if (value) {
+          r.attrs["_default"] = value.trim();
+        }
       } else {
-          key = match[4];
+        return null;
-        }
-
-        val = match[1] || match[5];
-
-        if (val) {
-          attrs[key] = val.trim().replace(/^["'“”](.*)["'“”]$/, "$1");
       }
+    } else if (r.attrs) {
+      r.attrs[key] = value?.trim() || "";
+    } else {
+      return null;
     }
   }
 
-    if (multiline && !trailingSpaceOnly(src, start + length, max)) {
+  if (r.tag) {
-      return;
+    if (multiline && !trailingSpaceOnly(src, start + bbcode.length + 1, max)) {
+      return null;
+    }
+    return r;
   }
 
-    tag = tag.toLowerCase();
+  return null;
-
-    return { tag, attrs, length };
-  }
 }
 
 function findBlockCloseTag(state, openTag, startLine, endLine) {
@@ -332,6 +384,7 @@ function applyBBCode(state, startLine, endLine, silent, md) {
 export function setup(helper) {
   helper.registerPlugin((md) => {
     isWhiteSpace = md.utils.isWhiteSpace;
+    escapeHtml = md.utils.escapeHtml;
 
     md.block.bbcode.ruler.push("excerpt", {
       tag: "excerpt",

app/assets/javascripts/discourse-markdown-it/src/features/d-wrap.js

@@ -1,41 +1,14 @@
-import { parseBBCodeTag } from "./bbcode-block";
+import { applyDataAttributes } from "./bbcode-block";
 
 const WRAP_CLASS = "d-wrap";
 
-function parseAttributes(tagInfo) {
-  const attributes = tagInfo.attrs._default || "";
-
-  return (
-    parseBBCodeTag(`[wrap wrap=${attributes}]`, 0, attributes.length + 12)
-      .attrs || {}
-  );
-}
-
-function camelCaseToDash(str) {
-  return str.replace(/([a-zA-Z])(?=[A-Z])/g, "$1-").toLowerCase();
-}
-
-function applyDataAttributes(token, state, attributes) {
-  Object.keys(attributes).forEach((tag) => {
-    const value = state.md.utils.escapeHtml(attributes[tag]);
-    tag = camelCaseToDash(
-      state.md.utils.escapeHtml(tag.replace(/[^A-Za-z\-0-9]/g, ""))
-    );
-
-    if (value && tag && tag.length > 1) {
-      token.attrs.push([`data-${tag}`, value]);
-    }
-  });
-}
-
 const blockRule = {
   tag: "wrap",
 
   before(state, tagInfo) {
     let token = state.push("wrap_open", "div", 1);
     token.attrs = [["class", WRAP_CLASS]];
-
+    applyDataAttributes(token, tagInfo.attrs, "wrap");
-    applyDataAttributes(token, state, parseAttributes(tagInfo));
   },
 
   after(state) {
@@ -49,8 +22,7 @@ const inlineRule = {
   replace(state, tagInfo, content) {
     let token = state.push("wrap_open", "span", 1);
     token.attrs = [["class", WRAP_CLASS]];
-
+    applyDataAttributes(token, tagInfo.attrs, "wrap");
-    applyDataAttributes(token, state, parseAttributes(tagInfo));
 
     if (content) {
       token = state.push("text", "", 0);
@@ -58,6 +30,7 @@ const inlineRule = {
     }
 
     state.push("wrap_close", "span", -1);
+
     return true;
   },
 };

app/assets/javascripts/discourse-markdown-it/src/features/text-post-process.js

@@ -1,3 +1,5 @@
+import { applyDataAttributes, parseBBCodeTag } from "./bbcode-block";
+
 export class TextPostProcessRuler {
   constructor() {
     this.rules = [];
@@ -59,7 +61,8 @@ export class TextPostProcessRuler {
         this.rules[i].rule.onMatch(
           buffer,
           match.slice(index, this.matcherIndex[i + 1]),
-          state
+          state,
+          { parseBBCodeTag, applyDataAttributes }
         );
         break;
       }

app/assets/javascripts/discourse/app/static/markdown-it/index.js

@@ -1,25 +1,9 @@
 import { htmlSafe } from "@ember/template";
-import { importSync } from "@embroider/macros";
-import loaderShim from "discourse-common/lib/loader-shim";
 import DiscourseMarkdownIt from "discourse-markdown-it";
 import loadPluginFeatures from "./features";
 import MentionsParser from "./mentions-parser";
 import buildOptions from "./options";
 
-// Shims the `parseBBCodeTag` utility function back to its old location. For
-// now, there is no deprecation with this as we don't have a new location for
-// them to import from (well, we do, but we don't want to expose the new code
-// to loader.js and we want to make sure the code is loaded lazily).
-//
-// TODO: find a new home for this – the code is rather small so we could just
-// throw it into the synchronous pretty-text package and call it good, but we
-// should probably look into why plugins are needing to call this utility in
-// the first place, and provide better infrastructure for registering bbcode
-// additions instead.
-loaderShim("pretty-text/engines/discourse-markdown/bbcode-block", () =>
-  importSync("./parse-bbcode-tag")
-);
-
 function buildEngine(options) {
   return DiscourseMarkdownIt.withCustomFeatures(
     loadPluginFeatures()

app/assets/javascripts/discourse/app/static/markdown-it/parse-bbcode-tag.js

@@ -1 +0,0 @@
-export { parseBBCodeTag } from "discourse-markdown-it/features/bbcode-block";

app/assets/javascripts/discourse/tests/unit/lib/parse-bbcode-tag-test.js

@@ -1,15 +0,0 @@
-import { setupTest } from "ember-qunit";
-import { module, test } from "qunit";
-import { parseBBCodeTag } from "discourse-markdown-it/features/bbcode-block";
-
-module("Unit | Utility | parseBBCodeTag", function (hooks) {
-  setupTest(hooks);
-
-  test("block with multiple quoted attributes", function (assert) {
-    const parsed = parseBBCodeTag('[test one="foo" two="bar bar"]', 0, 30);
-
-    assert.strictEqual(parsed.tag, "test");
-    assert.strictEqual(parsed.attrs.one, "foo");
-    assert.strictEqual(parsed.attrs.two, "bar bar");
-  });
-});

lib/pretty_text/shims.js

@@ -20,13 +20,6 @@ define("discourse-common/lib/helpers", ["exports"], function (exports) {
   };
 });
 
-define("pretty-text/engines/discourse-markdown/bbcode-block", [
-  "exports",
-  "discourse-markdown-it/features/bbcode-block",
-], function (exports, { parseBBCodeTag }) {
-  exports.parseBBCodeTag = parseBBCodeTag;
-});
-
 __emojiUnicodeReplacer = null;
 
 __setUnicode = function (replacements) {

plugins/discourse-local-dates/assets/javascripts/lib/discourse-markdown/discourse-local-dates.js

@@ -1,206 +1,113 @@
-import { parseBBCodeTag } from "pretty-text/engines/discourse-markdown/bbcode-block";
-
 moment.tz.link(["Asia/Kolkata|IST", "Asia/Seoul|KST", "Asia/Tokyo|JST"]);
 const timezoneNames = moment.tz.names();
 
-function addSingleLocalDate(buffer, state, config) {
+function addLocalDate(attributes, state, buffer, applyDataAttributes) {
+  if (attributes.timezone) {
+    if (!timezoneNames.includes(attributes.timezone)) {
+      delete attributes.timezone;
+    }
+  }
+
+  if (attributes.displayedTimezone) {
+    if (!timezoneNames.includes(attributes.displayedTimezone)) {
+      delete attributes.displayedTimezone;
+    }
+  }
+
+  if (attributes.timezones) {
+    attributes.timezones = attributes.timezones
+      .split("|")
+      .filter((tz) => timezoneNames.includes(tz))
+      .join("|");
+  }
+
+  const dateTime = moment.tz(
+    [attributes._default || attributes.date, attributes.time]
+      .filter(Boolean)
+      .join("T"),
+    attributes.timezone || "Etc/UTC"
+  );
+
+  const emailFormat =
+    state.md.options.discourse.datesEmailFormat || moment.defaultFormat;
+
+  attributes.emailPreview = `${dateTime.utc().format(emailFormat)} UTC`;
+
   let token = new state.Token("span_open", "span", 1);
-  token.attrs = [["data-date", state.md.utils.escapeHtml(config.date)]];
+  token.attrs = [["class", "discourse-local-date"]];
-
+  applyDataAttributes(token, attributes, "date");
-  if (!config.date.match(/\d{4}-\d{2}-\d{2}/)) {
-    closeBuffer(buffer, state, moment.invalid().format());
-    return;
-  }
-
-  if (config.time && !config.time.match(/\d{2}:\d{2}(?::\d{2})?/)) {
-    closeBuffer(buffer, state, moment.invalid().format());
-    return;
-  }
-
-  let dateTime = config.date;
-  if (config.time) {
-    token.attrs.push(["data-time", state.md.utils.escapeHtml(config.time)]);
-    dateTime = `${dateTime} ${config.time}`;
-  }
-
-  if (!moment(dateTime).isValid()) {
-    closeBuffer(buffer, state, moment.invalid().format());
-    return;
-  }
-
-  token.attrs.push(["class", "discourse-local-date"]);
-
-  if (config.format) {
-    token.attrs.push(["data-format", state.md.utils.escapeHtml(config.format)]);
-  }
-
-  if (config.countdown) {
-    token.attrs.push([
-      "data-countdown",
-      state.md.utils.escapeHtml(config.countdown),
-    ]);
-  }
-
-  if (config.calendar) {
-    token.attrs.push([
-      "data-calendar",
-      state.md.utils.escapeHtml(config.calendar),
-    ]);
-  }
-  if (config.range) {
-    token.attrs.push(["data-range", config.range]);
-  }
-
-  if (
-    config.displayedTimezone &&
-    timezoneNames.includes(config.displayedTimezone)
-  ) {
-    token.attrs.push([
-      "data-displayed-timezone",
-      state.md.utils.escapeHtml(config.displayedTimezone),
-    ]);
-  }
-
-  if (config.timezones) {
-    const timezones = config.timezones.split("|").filter((timezone) => {
-      return timezoneNames.includes(timezone);
-    });
-
-    token.attrs.push([
-      "data-timezones",
-      state.md.utils.escapeHtml(timezones.join("|")),
-    ]);
-  }
-
-  if (config.timezone && timezoneNames.includes(config.timezone)) {
-    token.attrs.push([
-      "data-timezone",
-      state.md.utils.escapeHtml(config.timezone),
-    ]);
-    dateTime = moment.tz(dateTime, config.timezone);
-  } else {
-    dateTime = moment.utc(dateTime);
-  }
-
-  if (config.recurring) {
-    token.attrs.push([
-      "data-recurring",
-      state.md.utils.escapeHtml(config.recurring),
-    ]);
-  }
-
   buffer.push(token);
 
-  const formattedDateTime = dateTime
-    .tz("Etc/UTC")
-    .format(
-      state.md.options.discourse.datesEmailFormat || moment.defaultFormat
-    );
-  token.attrs.push(["data-email-preview", `${formattedDateTime} UTC`]);
-
-  closeBuffer(buffer, state, dateTime.utc().format(config.format));
-}
-
-function defaultDateConfig() {
-  return {
-    date: null,
-    time: null,
-    timezone: null,
-    format: null,
-    timezones: null,
-    displayedTimezone: null,
-    countdown: null,
-    range: false,
-  };
-}
-
-function parseTagAttributes(tag) {
-  const matchString = tag.replace(/[‘’„“«»”]/g, '"');
-
-  return parseBBCodeTag(
-    "[date date" + matchString + "]",
-    0,
-    matchString.length + 12
-  );
-}
-
-function addLocalDate(buffer, matches, state) {
-  let config = defaultDateConfig();
-
-  const parsed = parseTagAttributes(matches[1]);
-
-  config.date = parsed.attrs.date;
-  config.format = parsed.attrs.format;
-  config.calendar = parsed.attrs.calendar;
-  config.time = parsed.attrs.time;
-  config.timezone = (parsed.attrs.timezone || "").trim();
-  config.recurring = parsed.attrs.recurring;
-  config.timezones = parsed.attrs.timezones;
-  config.displayedTimezone = parsed.attrs.displayedTimezone;
-  config.countdown = parsed.attrs.countdown;
-  addSingleLocalDate(buffer, state, config);
-}
-
-function addLocalRange(buffer, matches, state) {
-  let config = defaultDateConfig();
-  let date, time;
-  const parsed = parseTagAttributes(matches[1]);
-
-  config.format = parsed.attrs.format;
-  config.calendar = parsed.attrs.calendar;
-  config.timezone = (parsed.attrs.timezone || "").trim();
-  config.recurring = parsed.attrs.recurring;
-  config.timezones = parsed.attrs.timezones;
-  config.displayedTimezone = parsed.attrs.displayedTimezone;
-  config.countdown = parsed.attrs.countdown;
-
-  if (parsed.attrs.from) {
-    [date, time] = parsed.attrs.from.split("T");
-    config.date = date;
-    config.time = time;
-    config.range = "from";
-    addSingleLocalDate(buffer, state, config);
-  }
-  if (config.range) {
-    const token = new state.Token("text", "", 0);
-    token.content = "→";
-    buffer.push(token);
-  }
-  if (parsed.attrs.to) {
-    [date, time] = parsed.attrs.to.split("T");
-    config.date = date;
-    config.time = time;
-    config.range = "to";
-    addSingleLocalDate(buffer, state, config);
-  }
-}
-
-function closeBuffer(buffer, state, text) {
-  let token;
-
   token = new state.Token("text", "", 0);
-  token.content = text;
+  token.content = dateTime.utc().format(attributes.format);
   buffer.push(token);
 
   token = new state.Token("span_close", "span", -1);
-
   buffer.push(token);
 }
 
+function date(buffer, matches, state, { parseBBCodeTag, applyDataAttributes }) {
+  const parsed = parseBBCodeTag(matches[0], 0, matches[0].length);
+
+  if (parsed?.tag === "date") {
+    addLocalDate(parsed.attrs, state, buffer, applyDataAttributes);
+  } else {
+    let token = new state.Token("text", "", 0);
+    token.content = matches[0];
+    buffer.push(token);
+  }
+}
+
+function range(
+  buffer,
+  matches,
+  state,
+  { parseBBCodeTag, applyDataAttributes }
+) {
+  let token;
+  const parsed = parseBBCodeTag(matches[0], 0, matches[0].length);
+
+  if (parsed?.tag === "date-range") {
+    if (parsed.attrs.from) {
+      const { from, ...attributes } = { ...parsed.attrs, range: "from" };
+      delete attributes.to;
+      [attributes.date, attributes.time] = from.split("T");
+      addLocalDate(attributes, state, buffer, applyDataAttributes);
+    }
+
+    if (parsed.attrs.from && parsed.attrs.to) {
+      token = new state.Token("text", "", 0);
+      token.content = "→";
+      buffer.push(token);
+    }
+
+    if (parsed.attrs.to) {
+      const { to, ...attributes } = { ...parsed.attrs, range: "to" };
+      delete attributes.from;
+      [attributes.date, attributes.time] = to.split("T");
+      addLocalDate(attributes, state, buffer, applyDataAttributes);
+    }
+  } else {
+    token = new state.Token("text", "", 0);
+    token.content = matches[0];
+    buffer.push(token);
+  }
+}
+
 export function setup(helper) {
   helper.allowList([
     "span.discourse-local-date",
     "span[aria-label]",
-    "span[data-date]",
-    "span[data-time]",
-    "span[data-format]",
-    "span[data-countdown]",
     "span[data-calendar]",
+    "span[data-countdown]",
+    "span[data-date]",
     "span[data-displayed-timezone]",
+    "span[data-email-preview]",
+    "span[data-format]",
+    "span[data-recurring]",
+    "span[data-time]",
     "span[data-timezone]",
     "span[data-timezones]",
-    "span[data-recurring]",
-    "span[data-email-preview]",
   ]);
 
   helper.registerOptions((opts, siteSettings) => {
@@ -211,20 +118,14 @@ export function setup(helper) {
   });
 
   helper.registerPlugin((md) => {
-    const rule = {
+    md.core.textPostProcess.ruler.push("date", {
-      matcher: /\[date(=.+?)\]/,
+      matcher: /\[date=.+?\]/,
-      onMatch: addLocalDate,
+      onMatch: date,
-    };
-
-    md.core.textPostProcess.ruler.push("discourse-local-dates", rule);
     });
 
-  helper.registerPlugin((md) => {
+    md.core.textPostProcess.ruler.push("date-range", {
-    const rule = {
+      matcher: /\[date-range .+?\]/,
-      matcher: /\[date-range(.+?)\]/,
+      onMatch: range,
-      onMatch: addLocalRange,
+    });
-    };
-
-    md.core.textPostProcess.ruler.push("discourse-local-dates", rule);
   });
 }

plugins/discourse-local-dates/spec/lib/pretty_text_spec.rb

@@ -2,13 +2,13 @@
 
 def generate_html(text, opts = {})
   output = "<p><span"
-  output += " data-date=\"#{opts[:date]}\"" if opts[:date]
-  output += " data-time=\"#{opts[:time]}\"" if opts[:time]
   output += " class=\"discourse-local-date\""
-  output += " data-timezones=\"#{opts[:timezones]}\"" if opts[:timezones]
+  output += " data-date=\"#{opts[:date]}\"" if opts[:date]
-  output += " data-timezone=\"#{opts[:timezone]}\"" if opts[:timezone]
-  output += " data-format=\"#{opts[:format]}\"" if opts[:format]
   output += " data-email-preview=\"#{opts[:email_preview]}\"" if opts[:email_preview]
+  output += " data-format=\"#{opts[:format]}\"" if opts[:format]
+  output += " data-time=\"#{opts[:time]}\"" if opts[:time]
+  output += " data-timezone=\"#{opts[:timezone]}\"" if opts[:timezone]
+  output += " data-timezones=\"#{opts[:timezones]}\"" if opts[:timezones]
   output += ">"
   output += text
   output + "</span></p>"

spec/lib/cooked_post_processor_spec.rb

@@ -1906,13 +1906,12 @@ RSpec.describe CookedPostProcessor do
     end
 
     context "with an unmodified quote" do
-      let(:cp) do
+      let(:cp) { Fabricate(:post, raw: <<~MARKDOWN) }
-        Fabricate(
+        [quote="#{pp.user.username}, post: #{pp.post_number}, topic:#{pp.topic_id}"]
-          :post,
+        ripe for quoting
-          raw:
+        [/quote]
-            "[quote=\"#{pp.user.username}, post: #{pp.post_number}, topic:#{pp.topic_id}]\nripe for quoting\n[/quote]\ntest",
+        test
-        )
+      MARKDOWN
-      end
 
       it "should not be marked as modified" do
         cpp.post_process_quotes
@@ -1921,13 +1920,12 @@ RSpec.describe CookedPostProcessor do
     end
 
     context "with a modified quote" do
-      let(:cp) do
+      let(:cp) { Fabricate(:post, raw: <<~MARKDOWN) }
-        Fabricate(
+        [quote="#{pp.user.username}, post: #{pp.post_number}, topic:#{pp.topic_id}"]
-          :post,
+        modified
-          raw:
+        [/quote]
-            "[quote=\"#{pp.user.username}, post: #{pp.post_number}, topic:#{pp.topic_id}]\nmodified\n[/quote]\ntest",
+        test
-        )
+      MARKDOWN
-      end
 
       it "should be marked as modified" do
         cpp.post_process_quotes
@@ -1936,13 +1934,12 @@ RSpec.describe CookedPostProcessor do
     end
 
     context "with external discourse instance quote" do
-      let(:external_raw) { <<~RAW.strip }
+      let(:cp) { Fabricate(:post, user: user_with_auto_groups, raw: <<~MARKDOWN.strip) }
         [quote="random_guy_not_from_our_discourse, post:2004, topic:401"]
         this quote is not from our discourse
         [/quote]
         and this is a reply
-        RAW
+      MARKDOWN
-      let(:cp) { Fabricate(:post, user: user_with_auto_groups, raw: external_raw) }
 
       it "it should be marked as missing" do
         cpp.post_process_quotes

spec/lib/pretty_text_spec.rb

@@ -2633,7 +2633,7 @@ HTML
       cooked = PrettyText.cook("Hello [wrap=toc id=1]taco[/wrap] world")
 
       html = <<~HTML
-        <p>Hello <span class="d-wrap" data-wrap="toc" data-id="1">taco</span> world</p>
+        <p>Hello <span class="d-wrap" data-id="1" data-wrap="toc">taco</span> world</p>
       HTML
 
       expect(cooked).to eq(html.strip)
@@ -2644,7 +2644,7 @@ HTML
       SiteSetting.enable_markdown_typographer = true
 
       md = <<~MD
-        [wrap=toc id="a” aa='b"' bb="f'"]
+        [wrap=toc id=“a” aa='b"' bb="f'"]
         taco1
         [/wrap]
       MD
@@ -2652,7 +2652,7 @@ HTML
       cooked = PrettyText.cook(md)
 
       html = <<~HTML
-        <div class="d-wrap" data-wrap="toc" data-id="a" data-aa="b&amp;quot;" data-bb="f'">
+        <div class="d-wrap" data-aa="b&amp;quot;" data-bb="f'" data-id="a" data-wrap="toc">
         <p>taco1</p>
         </div>
       HTML
@@ -2679,7 +2679,7 @@ HTML
       cooked = PrettyText.cook("[wrap=toc name=\"single quote's\" id='1\"2']taco[/wrap]")
 
       html = <<~HTML
-        <div class="d-wrap" data-wrap="toc" data-name="single quote's" data-id="1&amp;quot;2">
+        <div class="d-wrap" data-id="1&amp;quot;2" data-name="single quote's" data-wrap="toc">
         <p>taco</p>
         </div>
       HTML
@@ -2691,7 +2691,7 @@ HTML
       cooked = PrettyText.cook('[wrap=toc foo="<script>console.log(1)</script>"]taco[/wrap]')
 
       html = <<~HTML
-        <div class="d-wrap" data-wrap="toc" data-foo="&amp;lt;script&amp;gt;console.log(1)&amp;lt;/script&amp;gt;">
+        <div class="d-wrap" data-foo="&amp;lt;script&amp;gt;console.log(1)&amp;lt;/script&amp;gt;" data-wrap="toc">
         <p>taco</p>
         </div>
       HTML
@@ -2703,9 +2703,7 @@ HTML
       cooked = PrettyText.cook('[wrap=toc fo@"èk-"!io=bar]taco[/wrap]')
 
       html = <<~HTML
-        <div class=\"d-wrap\" data-wrap=\"toc\" data-io=\"bar\">
+        <p>[wrap=toc fo@"èk-"!io=bar]taco[/wrap]</p>
-        <p>taco</p>
-        </div>
       HTML
 
       expect(cooked).to eq(html.strip)