- Issue created by @pameeela
- 🇦🇺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.
- 🇺🇸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.
- 🇦🇺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.
- 🇬🇧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.
- 🇦🇺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.
- 🇺🇸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.
- 🇺🇸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.
- 🇺🇸United States phenaproxima Massachusetts
Okay, I think this is pretty much where it needs to be. Reassigning to @pameeela for final review.
-
phenaproxima →
committed 02135f9a on 0.x authored by
pameeela →
Issue #3486284 by phenaproxima, pameeela, tonypaulbarker: Create a new...
-
phenaproxima →
committed 02135f9a on 0.x authored by
pameeela →
Automatically closed - issue fixed for 2 weeks with no activity.