- Issue created by @Dom.
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 5:35pm 8 May 2023 - πΊπΈ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 3:20pm 9 May 2023 - πΊπΈ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| | |<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 6:29pm 12 May 2023 - πΊπΈ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 9:37am 18 July 2023 Automatically closed - issue fixed for 2 weeks with no activity.