backdrop-issues icon indicating copy to clipboard operation
backdrop-issues copied to clipboard

Filter: url filter extended settings lost when saving the form without changes

Open indigoxela opened this issue 1 year ago • 3 comments

Description of the bug

Not sure, when this broke, but currently the extended filter settings for the URL filter are lost, if they're not adapted before saving the form.

Discovered the problem with the Video Filter module, but the core url filter's also affected, so reporting it here.

Steps To Reproduce

  1. Go to admin/config/content/formats/filtered_html
  2. Enable the "Convert URLs into links" (on by default)
  3. Open its configure form (a dialog)
  4. Change the url length to a different value
  5. Save and save the filter form
  6. Check that the json config for that filter has the value
  7. Open the form on admin/config/content/formats/filtered_html again
  8. Save the form (without touching the url filter settings)
  9. Verify that the setting for filter_url_length is back to defaults (the next time opening the form)
  10. Verify that the config (Text formats / Basic) filter_url / settings is an empty array

Actual behavior

This bug affects "Convert URLs into links", but does not affect "allowed_html". The latter keeps its settings. Probably because of the special handling for allowed_html in function filter_admin_format_form(). Other filters, not treated specially there, all lose all their settings on second save - as their sub-form's not called then.

Expected behavior

Settings not lost, just because I saved the format for other reasons. :wink:

Additional information

Add any other information that could help, such as:

  • Backdrop CMS version: latest dev
  • Web server and its version: Apache
  • PHP version: 8.2

indigoxela avatar Mar 21 '24 13:03 indigoxela

So, here's the deal: in filter_admin_format_form_submit() the original values from the extended "Configure" forms are picked from the tempstore. But ... if none of the AJAX forms (dialog) has been opened, the tempstore isn't even initialized, so we lose all previous extended filter settings.

A PR is available for testing and review.

How to test:

  • Open any format form, like admin/config/content/formats/plain_text - it doesn't matter which
  • Enable the url filter (Convert URLs into links)
  • Click on "Configure" and set the length value to something not default
  • Save the form
  • Open the form again and save it without touching any of the dialogs
  • Go to admin/config/development/configuration/single/export and check the filter settings json for the format
  • Repeat the last two steps several times

The configured filter settings should now stay in place. Just ignore filter_html, those values get handled pretty differently.

Core only ships with two extended forms, the one for filter_url and the one for filter_html. The latter's not affected by this bug. So in case you're testing locally, you might also consider testing with the contrib "Video filter" module, which was the initial trigger for this bug report here.

indigoxela avatar Mar 22 '24 15:03 indigoxela

Comprehensive instructions, @indigoxela. I'll give it a test.

herbdool avatar Mar 22 '24 16:03 herbdool

This looks good. I've tested both with and without the PR. And reviewed.

herbdool avatar Mar 23 '24 12:03 herbdool

Wow, this is a pretty big potential issue. Thanks @indigoxela for the fix and @herbdool for reviewing! I tested and confirmed the problem and that the PR fixes it. Merged https://github.com/backdrop/backdrop/pull/4682 into 1.x and 1.27.x. Thank you @indigoxela!

quicksketch avatar Apr 28 '24 03:04 quicksketch