Proposed resolution

Add support for the 3rd party "Footnotes" extension: https://github.com/rezozero/commonmark-ext-footnotes

N/A

Plugin ID rezozero/commonmark-ext-footnotes
Settings Schema
Settings Key N/A
Settings N/A

Remaining tasks

  • Create MarkdownExtension plugin.
CommentFileSizeAuthor
#5 3131224-5.patch934 bytesmikejw
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markcarver created an issue. See original summary.

  • markcarver committed 3c2c15b on 8.x-2.x 
    Issue #3131224 by markcarver: CommonMark 3rd-party Extension: Footnotes...
markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Status: Active » Fixed
mikejw’s picture

mikejw CreditAttribution: mikejw commented
Status: Fixed » Needs work

Can't yet get this to work using the filter output strategy. The problem is due to https://www.drupal.org/project/drupal/issues/2544110 as inside the `sup` tag that the extension puts in, it puts in an `id` attribute with a colon e.g. "fnref:note1" and the Xss filtering strips out the `fnref` part.

One approach might be a PR / feature request on the extension to make the prefix configurable and then one could leave out the colon altogether.

Also, need to change the rest of the allowedHtmlTags since some of the bottom markup that is injected is filtered out, will post a patch in a minute.

mikejw’s picture

mikejw CreditAttribution: mikejw commented
FileSize
3131224-5.patch934 bytes

This patch won't fix it, but the allowedHtmlTags will need to be extended either way.

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented

I didn't run into this issue at all when implementing it. Are you sure you have the global "Markdown" allowed HTML plugin enabled? It should be supplying the base allowed HTML.

mikejw’s picture

mikejw CreditAttribution: mikejw commented

Interesting - not sure I follow what you are saying around the global html plugin. I have it setup as follows:
- filter with only markdown enabled
- parser set to commonmark
- render strategy set to filter, with:
- markdown module enabled (is this what you are referring to?)
- footnotes parser extension enabled
- footnotes enabled

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Status: Needs work » Fixed

So the id prefix stripping of fn: and fnref: are indeed a core issue. Due to the lack of configurability with the extension, there's really not much I can do about this ATM.

Temporary workaround for users is to implement a custom hook_markdown_html_alter() to remove any additional occurrances of fn: and fnref: in the link href.

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Related issues: +#2544110: XSS attribute filtering is inconsistent and strips valid attributes
markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented

Example of the temporary workaround mentioned in #8:


/**
 * Implements hook_markdown_html_alter().
 */
function MY_MODULE_markdown_html_alter(&$html, array $context) {
  $parser = $context['parser'];
  if (!($parser instanceof \Drupal\markdown\Plugin\Markdown\CommonMark\CommonMark) || !$parser->extensions()->has('rezozero/commonmark-ext-footnotes') || !$parser->extension('rezozero/commonmark-ext-footnotes')->isEnabled()) {
    return;
  }
  // Replace footnotes extension prefixes so they don't contain colons.
  // They are stripped due to a core XSS bug.
  // @see https://www.drupal.org/project/drupal/issues/2544110
  // @see https://www.drupal.org/project/markdown/issues/3131224#comment-13613381
  $html = str_replace('fn:', 'fn-', $html);
  $html = str_replace('fnref:', 'fnref-', $html);
}

  • markcarver committed 78f233c on 8.x-2.x 
    Issue #3131224 by markcarver, mikejw: CommonMark 3rd-party Extension:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.