Create a new text format/editor to replace core's Basic HTML

Created on 8 November 2024, about 1 month ago

Problem/Motivation

We are currently using Basic HTML editor from core as the only one in Drupal CMS. It is very limited and lacks a few things people expect.

Proposed resolution

Create a new format and include this in the base recipe.

Remaining tasks

  1. Create the text format
  2. Update the base recipe to use it
📌 Task
Status

Active

Component

Base Recipe

Created by

🇦🇺Australia pameeela

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

Merge Requests

Comments & Activities

  • Issue created by @pameeela
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 418s
    #332677
  • 🇦🇺Australia pameeela

    I added the new format and updated most of the references, but not sure what the situation is with privacy and content type base using the basic_html, I'm guessing there's a reason for this and I'm not sure how to resolve it.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 353s
    #332682
  • 🇺🇸United States phenaproxima Massachusetts

    So the reason that content_type_base (and thus privacy) use core formats is because they are applied before the starter recipe. Not only that, but they could be applied independently of the starter recipe and therefore need a format that they have ironclad, guaranteed access to.

    Now, having said that, I just took a look at core and they have thought of this.

    If a text field value is using an unavailable format (or unspecified), the value will be rendered using the fallback format, which is defined in config. It defaults to plain_text, which is a filter format that ships with the Filter module.

    So here's what I recommend we do:

    • Ensure the plain_text filter format is installed by explicitly having content_type_base import it.
    • In all the default content we ship, just delete the lines that say format: whatever. That oughta force the system to fall back to the default format, which will be plain_text unless we change it.
    • So, in the drupal_cms_starter recipe, change the fallback format to your new default format, so that content with an unspecified format will use it.
  • 🇦🇺Australia pameeela

    That sounds like a plan, if it works! I noticed that we no longer even had the plain text format so was wondering what was up with that. FWIW email has also added another format so the whole 'we should only have one format' thing is kinda out the window anyway.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 754s
    #339446
  • 🇦🇺Australia pameeela

    Bumping this to a major bug because the core basic HTML format is missing key Drupal CMS features such as media library and linkit, so this is a big regression.

    Actioned your suggestion and it seems to work.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 570s
    #339466
  • 🇬🇧United Kingdom tonypaulbarker Leeds

    I support this. It will solve an instance of the same problem that I was having 🐛 Editor and Filter configuration from drupal_cms_starter is not applied Active . I think we could name it something more meaningful than 'default' but deciding on the best naming shouldn't hold us up.

  • 🇦🇺Australia pameeela

    Yeah, I was unsure of the name but I feel strongly that it should not have “HTML” in it or be otherwise a relative or subjective description like “restricted” or “basic”. I think “default” is better than that at least!

  • 🇬🇧United Kingdom tonypaulbarker Leeds

    Throwing some ideas in.. if we want it to describe what’s included -> ‘Rich text and media’ ‘Web text and media’ or if not, since we have an email profile ‘webpage format’,
    ‘Content format’, ‘default_formatting’, ‘standard_format’, ‘standard_editor_format’. We could also describe that it comes from Drupal CMS: ‘cms_editor_format’

    The trouble with ‘default’ is that it is used in so many places that it doesn’t tell me it’s an editor profile or anything about what’s included when I select it and since it’s not at all unique I can’t search the codebase on it.

  • 🇦🇺Australia pameeela

    I'm fine with 'Content' for the label and 'content_format' for the machine name. This distinguishes somewhat from 'Email' in the list, but doesn't make any subjective judgements.

  • Merge request !207Resolve #3486284 "Text format" → (Merged) created by pameeela
  • Pipeline finished with Failed
    about 1 month ago
    Total: 465s
    #341482
  • 🇦🇺Australia pameeela

    Oh, I forgot to create an MR for this, that explains why it didn't get a look :)

    There are some bad merge conflicts since the repo was reorganised after this so I didn't try to rebase because I think this will make more sense to you @phenaproxima.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 495s
    #341488
  • Pipeline finished with Failed
    about 1 month ago
    #341487
  • Pipeline finished with Failed
    about 1 month ago
    Total: 73s
    #342155
  • 🇺🇸United States phenaproxima Massachusetts

    OK - that was a hell of a rebase but I got it done.

    It turns out my plan in #3 won't work as expected due to some technical stuff in the Filter module (which is frankly a weird mess and needs to be heavily refactored, but that's not for us to do).

    I now think what makes the most sense here, especially in light of the very clear "Content" naming, and the fact that there are also an "Email" and "Webform" formats, is to ship the new format in drupal_cms_content_type_base. That way we know it's available to all content types, since they all depend on that base recipe.

  • Pipeline finished with Canceled
    about 1 month ago
    Total: 128s
    #342162
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 158s
    #342166
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 64s
    #342171
  • Pipeline finished with Failed
    about 1 month ago
    Total: 718s
    #342173
  • 🇺🇸United States phenaproxima Massachusetts

    I ran into a core bug. If you apply content_type_base on Standard without any media types existing, you will get this error when you try to add content:

    The website encountered an unexpected error. Try again later.

    InvalidArgumentException: The allowed types parameter is required and must be an array of strings. in Drupal\media_library\MediaLibraryState->validateRequiredParameters() (line 151 of core/modules/media_library/src/MediaLibraryState.php).

    This is unambiguously a bug in core. The MediaLibraryState object should not be resilient to the possibility that no media types exist. We'll have to open an issue for that.

    In the meantime, this is probably not a commit blocker or stable blocker, because we don't normally expect folks to apply drupal_cms_content_type_base on top of Standard, although technically we need to support it. The workaround is probably to get the tests passing by applying a media type recipe (drupal_cms_image_media_type) when setting up the test site.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 754s
    #342192
  • Pipeline finished with Failed
    about 1 month ago
    Total: 871s
    #342217
  • 🇺🇸United States phenaproxima Massachusetts

    Okay, I think this is pretty much where it needs to be. Reassigning to @pameeela for final review.

  • Pipeline finished with Success
    about 1 month ago
    Total: 876s
    #342267
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1093s
    #342502
  • 🇦🇺Australia pameeela

    Awesome, thanks!

  • Pipeline finished with Skipped
    about 1 month ago
    #342777
  • 🇺🇸United States phenaproxima Massachusetts

    Thanks! Merged into 0.x.

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

Production build 0.71.5 2024