The BaseParser::getRenderStrategy() does not get the parser's configured render strategy

Created on 1 June 2023, over 1 year ago
Updated 23 October 2023, over 1 year ago

Problem/Motivation

When using the markdown service and parser method in custom logic, I observed that the returned parsed content is stripped of markup (see related issue [#]). When I debugged the logic, I found that the the BaseParser::getRenderStrategy() does not get the parser's configured render strategy. Instead it uses the fallback default static::FILTER_OUTPUT.

I observed that saving the parser config generates a config object/file of name markdown.parser.commonmark, which includes the configured render strategy type of none. I then observed that InstallablePluginBase::getConfigurationName() logic produces a config name of installable.plugin.markdown_commonmark, which does not match the configuration created when saving the parser form. I'm not sure this is related, but wanted to share this.

I think this is a symptom of the underlying issue, which is the parser isn't instantiated with its configuration. As a result, all HTML is being stripped. Though, I wonder if html should be stripped prior to converting the markdown.

Steps to reproduce

  • Use composer to require drupal/markdown:3.0.0-rc2 and league/commonmark:1.6.7
  • Enable markdown module
  • Go to /admin/config/content/markdown
  • Disable the "CommonMark GFM" parser (I did this to only have one enabled parser)
  • Edit the CommonMark (default) parser
  • Select the "Render Strategy" vertical tab
  • Choose "None"
  • Save configuration
  • Execute Drush command to invoke parser and catch exception, drush php-eval "echo(\Drupal::service('markdown')->parse('This is **BOLD**.'));"
  • Observe output: "This is BOLD."
  • Expected: "<p>This is <strong>BOLD</strong>.</p>"

Proposed resolution

TBD

Remaining tasks

TBD

User interface changes

TBD

API changes

TBD

Data model changes

TBD

πŸ› Bug report
Status

Needs review

Version

3.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @jasonawant
  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA
  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

    Regarding

    Another observation related to BaseParser::parse() method is that when using the fallback filter_output render strategy type, all markup is stripped, including markdown converted to HTML b/c this occurs after the conversion here within BaseParser::parse().

    ,

    the order could be swapped to first filter out user provided HTML before converting markdown into HTML.

    This could be a separate issue.

  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

    Ah, I think I figured out a work around.

    Instead of invoking \Drupal::service('markdown')->parse(), which uses the default parser; l instantiated a parser by name with the parser's configuration. For example:

     drush php-eval "echo(\Drupal::service('markdown')->getParser('commonmark', Drupal::config('markdown.parser.commonmark')->getRawData())->parse('This is **BOLD**.'));"
    

    This results in the expected output of
    <p>This is <strong>BOLD</strong>.</p>

  • First commit to issue fork.
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    Build Successful
  • @g-brodiei opened merge request.
  • Status changed to Needs review over 1 year ago
  • πŸ‡ΉπŸ‡ΌTaiwan g-brodiei

    I have encountered the same issue and scenario as OP, after looking into this code. I've made change to assure the renderStrategy string may be fetched from the correct source, whether it's from the Plugin Configuration or the settings object.

    Settings to needs review.

  • πŸ‡ΉπŸ‡ΌTaiwan g-brodiei

    This should correct the behavior expected from the module's documentation β†’ .

Production build 0.71.5 2024