Improve validation on text format configuration

All tests passing.

I have tested this change, and it generates an error when saving the filter if "convert line breaks" and "limited HTML tags" filters aren't enabled.

I think this is an improvement overall.

I will note that as long as the required filters come after MarkDown easy, it doesn't care what order they go in. As far as I understand it will behave a bit funny if "Convert line breaks" isn't the last filter. Would it be worth having an warning about the order of those filters?

If I'm not mistaken, this change makes the two filters "required" rather than "strongly recommended". Could there be cases where someone wants to use it without those filter, such as if they were using a custom filter that served the same purpose? Should there be a checkbox to say "I know you recommend those filters, but I want to do it anyway"?

@lostcarpark - Thanks for the feedback. I've added additional validation (and test) to ensure "Limit allowed HTML" runs before "Convert line feeds".

Regarding your suggested "I know you recommended..." checkbox, I understand where you're coming from, but rather than providing a checkbox, I will provide in both the module documentation and README file instructions on how to disable the additional validation via a hook implementation.

-mike

I'm fully on board with disabling the validation through a hook.

I'm getting slightly funny behaviour with the validation. I'm not sure if the problem is with the validation or with the Text format admin page in core.

Here's what's happening for me...

1. Edit a text format with the markdown filter (and the "limit HTML" and "convert line breaks" in the correct order).
2. Switch the order of "limit HTML" and "convert line feeds" and press save.
3. Validation error occurs.
4. Form redisplays with "limit HTML" above "convert line feeds", so no change appears to be required.
5. Press save again. Same validation message appears.
6. Reorder "limit HTML" and "convert line feeds", but then change them back to correct order.
7. Press save, and form saves correctly.

What seems to be happening, is when the validation fails, the form redisplays, but reverts to the previously saved order for display. However, it would seem that the changed order is still held on the form.

When I first saw the items reorder on validation fail, it just seemed a little disconcerting, but if the order is still stored but not displayed, that could be a much bigger problem.

It's possible there needs to be some form alter code to force the items into the correct order.

I installed and configured the module. I edited a text format with the following markdown filters: markdown easy, limit allowed HTML tags and correct faulty HTML, and convert line breaks into HTML. I tried to rearrange the "Markdown Easy filter" below the "limit allowed HTML tags and correct faulty HTML" and "Convert line breaks into HTML" filters and pressed save and received an error message saying, "The Markdown Easy filter needs to be placed before the Limit allowed HTML tags and correct faulty HTML filter" and the arrangement of the filters was changed so the "Markdown Easy filter" was above the "limit allowed HTML tags and correct faulty HTML" and "convert line breaks into HTML" filters. Even though the arrangement of the filters was now in the correct order I pressed save again and I received the same error message. I had to manually rearrange the filters so the "Markdown Easy" filter is on top again with unsaved changes. Once I saved the changes I received no error messages.

@lostcarpark, @cindy_codediaries - I know exactly what you are talking about. You can actually see the issue better if you “Show row weights” after the first time validation fails - the order doesn’t match the values.

This isn't anything that the new validation code is doing, I'm pretty sure this is Drupal core's default behavior when setting an error on series of checkboxes that also have weights for each enabled item. I have confirmed this by spinning up a site on simplytest.me and confirming this behavior occurs with the "Embed media" filter's validation (which Markdown Easy's validation is based on).

Therefore, I've opened a core issue for this: 🐛 Validation of text filter order results in unexpected results Active

I don't think this issue should stop this from being merged - thoughts?

-mike

I don't think so either. My vote is yes merge.

I agree this looks like a problem with the core code, that doesn't manifest in the default form because there is no validation on filter order.

What seems to be happening is the submitted weights are not getting applied when the form is displayed.

I think if merged in its current state, it could cause confusion, especially for novice users.

I've come up with a fix to consider. Basically, in the hook_form_alter, it copies the weights from the submitted values to the filter order table on the form, then sorts that table by the weights.

I'm not sure whether we should put this fix for a core issue in the module, but I do think it's the module's form alteration that's causing the core bug to manifest. We'd need to keep an eye on that issue, and possibly remove or modify this code when it gets fixed. However, core issues can move quite slowly, so we could be living with this behaviour for quite a while.

I installed and followed the steps to test this module on my local. It seems working well. If I config the Markdown Easy filter below the "Limit allowed HTML tags and correct faulty HTML" and "Convert line breaks into HTML" filters. I will get Warning message which is super helpful.

suggestion : for the commit with id=0cdd9537 we can remove the "else block" in this file FormStateInterface; and keep just the "return" statement.

Thanks everyone! Merging.

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