Make "doNotConsiderEmpty" tags configurable.

Created on 8 May 2023, over 1 year ago
Updated 18 July 2023, over 1 year ago

After πŸ“Œ Adding nbsp; removing + better prep_replace pattern Fixed a variable was introduced to consider some tags as not filterable, hence not considered empty.
The list is currently hardcoded in the module. The purpose of this issue is to get that configurable.

✨ Feature request
Status

Fixed

Version

1.0

Component

Code

Created by

πŸ‡«πŸ‡·France Dom.

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

Comments & Activities

  • Issue created by @Dom.
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    Hi, Dom.,
    The patch looks like a great start. Here's a screenshot of the form just to give everyone a sense of what's happening.

  • Assigned to danflanagan8
  • Status changed to Needs work over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    Oops, I meant to embed that...

    And here's what I get in the config based on those settings:

      filter_empty_tags:
        id: filter_empty_tags
        provider: filter_empty_tags
        status: true
        weight: 0
        settings:
          do_not_consider_empty: 'button canvas drupal-media drupal-entity iframe object script svg textarea td th'
          filter_spaces: '1'
          filter_nbsp: '1'
          filter_br: '1'
          pattern: '/<([^<\/>)]*)\b[^>]*>((\s| |&nbsp;|<br>|<br\/>|<br \/>)*?)<\/\1>/imsU'
    

    It doesn't seem to be filtering a paragraph containing a non-breaking space for me though in my manual testing. Which is weird because the automated tests all pass.

    This may have something to do with the unusual character you called out on the big non-breaking space issue. I'll investigate that.

    I'm also interested in adding tests cases for the various combinations of settings. That's something I can hopefully get to in the next few days.

  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    Here's a patch with a pretty big diff. Here are the important changes.

    1. I fixed the problem I was having with unicode non-breaking spaces
    2. I added schema
    3. I removed the pattern setting in favor of building it more often. I don't think this will significantly affect performance and it prevents the possibility of the pattern getting out os sync with the other settings if somehow config changes are made outside the context of the form.
    4. I added a bit of test coverage for the new settings. That required refactoring the unit tests so that the configuration could be passed from the dataProvider.

    My manual testing was positive. We'll see if tests pass here.

    I'm also wondering if we'll need an update hook to go from 1.x. to 2.x. I don't think so because there are default settings in the plugin annotation.

  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States danflanagan8 St. Louis, US

    > We'll see if tests pass here.

    I guess they're not configured to run on d.o yet. The first step is to make official dev releases. I don't have permission to do that. It looks like I have permission to set up tests though if those releases were made.

  • πŸ‡«πŸ‡·France Dom.

    Review here:
    1- the patch did not applied for some reasons. Attached is the new updated patch.
    2- Added test coverage to check cases when br, nbsp and space should not be filtered
    3- Added an empty hook to triggered a cache rebuild => necessary for the default configuration to be accounted when update from 1.x to 2.x

  • Status changed to Fixed over 1 year ago
    • Dom. β†’ committed 9934c3bd on 2.x
      Issue #3358924 by Dom., danflanagan8: Make "doNotConsiderEmpty" tags...
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024