🇨🇦Canada @star-szr

star-szr → changed the visibility of the branch 3525010-allow-belgrade-to to hidden.

star-szr → created an issue.

Late to the party but we are trying to use content_sync with Commerce, so created 📌 Graceful handling of serialization failures during export Active as a way forward. Please review and test the code changes there. Thanks!

Sometimes we log/display an error, other times a warning. Suggestions welcome there, I went with warning for the non-user-initiated exports (entity hooks) because it seemed too noisy to log errors each time for that.

With these changes I have tested:

• UI Single export
• UI Archive export
• Entity insert and entity update operations
• Drush export (content-sync-export)

This testing was performed with Drupal 11 on top of the latest changes in ✨ Drupal 11 compatibility Active but should work just fine in Drupal 9 and 10.

Sorry for the noise, adding the one I mentioned previously as related.

star-szr → created an issue.

For the most recent error see also 🐛 FileEntityNormalizer::denormalize(): Return value must be of type ArrayObject|array|string|int|float|bool|null Active .

Edit: although looking at the fix there, it's not doing the right thing in my opinion.

Regarding the serializers, this old issue seems worth pointing out: #3255826: Leverage Core Serializers →

This older issue seems related and has a patch (though it may be outdated): ✨ Add config to be able to ignore entity types Needs review

@kul.pratap I believe when you commented that an issue fork with changes had already been started by @ergonlogic so this may not be the best issue to jump into since we are already actively working on it. I'm going to go ahead and unassign for now. Thank you for wanting to contribute!

This change record seems relevant for changes required for the normalizers: https://www.drupal.org/node/3359695 →

Re-titling to make it easier to tell this apart from 🐛 Update getConfigImporter for Drupal 11 compatibility Active .

Going to add another fix and try to make this a general D11 compatibility issue.

ergonlogic → credited star-szr → .

Drupal 7 has reached end of life as of January 5, 2025, going to be bold and close this.

https://www.drupal.org/psa-2023-06-07 →

star-szr → created an issue.

star-szr → created an issue. See original summary → .

This is ready for review/testing and I've updated the issue summary.

The tests are running successfully in GitLab CI now with the work from 📌 Fix PHP deprecations Active merged.

My next steps are:

• Expand the test coverage to include the "ignore" part of the functionality and anything else that may be missing
• Enhance the UI/better incorporate the checkbox

I also ran phpstan on config_enforce and didn't see any similar errors.

I've made a note to create issues on both projects to resolve remaining phpstan errors as a separate piece.

Digging more into the phpstan results and there are a number of not-very-straightforward refactors that I'm seeing which would also be risky with our current level of automated testing.

I am going to stick to just fixing the PHP deprecations here to unblock the related issue.

star-szr → created an issue.

I have pushed code that adds a .local-dev DDEV environment and basic Behat setup. Drumkit hasn't been fully incorporated yet, from my understanding files just need to be added to drumkit/mk.d.

For the commits that were the result of running a command I have documented these in the commits, you can see them with the commit hashes in reverse chronological order by running git log --format="%h %(trailers:key=Command,valueonly)" --grep="Command: "

star-szr → made their first commit to this issue’s fork.

I appreciate the feedback!

I have added a 'Usage' section on the project page, feedback welcome.

Sorry for the trouble, I wish this was more standardized.

I added a link to “View README” in the project sidebar under Resources, what do you think?

While working on tests, spotted this:

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for config_enforce_devel.settings with the following errors: config_enforce_devel.settings:defaults.config_directory missing schema, config_enforce_devel.settings:defaults.enforcement_level missing schema, config_enforce_devel.settings:defaults.enforce_dependencies missing schema, config_enforce_devel.settings:ignored_configs missing schema in Drupal\Core\Config\Development\ConfigSchemaChecker->onConfigSave() (line 94 of core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php).

Noting another TODO: The /opt/project mount might not be needed with this setup, since our contrib project is available at /var/www/html and we could presumably use that as our composer symlink.

Noting a TODO: "make tests" is taking a snapshot, maybe we can use DDEV snapshots for this instead.

star-szr → created an issue.

star-szr → made their first commit to this issue’s fork.

I've hit retry three times on the test, but it's still randomly failing. Not your fault @joel_osc, thanks for your work on this! See 🌱 Replace Nightwatch testing with another solution Active .

I think this feature also needs additional test coverage to:

1. Ensure that the Description field saves and displays properly
2. Explicitly test for the "empty row" behaviour (row should be deleted if Description is non-empty but Search and Replace are empty)

Sadly those tests would need to be ported to Playwright later if we write them in Nightwatch now, but I won't merge this without test coverage. If nobody comes back to this I will add those tests at a later date, but I will be working on 🌱 Replace Nightwatch testing with another solution Active first.

At this point the path seems clear to use Playwright, of course things can change but converting to a task for now.

I am hoping to have some time available in the next week or two to continue this work.

I re-ran the Nightwatch tests (it tends to randomly fail, I am working on replacing it), looks like there is one phpcs error left. Thanks!

@joel_osc I am leaning that way as well, thanks for weighing in. Could you please take a look at the phpcs errors in the .install?

Pushed a commit that should resolve the test failures, it was due to the config in the test module not being updated with the new description field.

I think at least one other thing that deserves some thought is the behaviour for deleting rows, should we check that all three text fields are empty? Currently we just check that the search and replace are empty and delete the filter row if they are both empty. I think we should probably include the description field and update the messaging/tests/etc. accordingly, but open to other suggestions and thoughts here.

Thanks @joel_osc, I will take a look.

Hi @joel_osc thanks for this!

Could you please attach the patch/MR and any additional details to ✨ Add text field to paste filters for documentation Active ?

I'm not going to create a new release for this since it's just internal tooling/documentation, but this is done! I should have created this issue earlier but wanted to have a record of it at least.

star-szr → created an issue.

drumm → credited star-szr → .

As the module maintainer in question, this was certainly an unpleasant surprise.

Even linking to some docs would be an improvement, it wasn’t clear to me that these checkboxes would have any impact beyond the Drupal.org project page.

I have set 1.0.1 back to “Supported”. Thanks for the report!

Hmm, my fault for not reading the docs I suppose, it wasn’t clear to me that this would happen.

Sorry for the alarm/noise. My intent was simply to only point to one version on the project page, but I guess that’s not really an option.

I did a quick search today and found that core is looking to go in the direction of Playwright, which based on my recent evaluations for a client project would be very high on my list as well.

Tools to check out:

https://github.com/Lullabot/ddev-playwright
https://github.com/julienloizelet/ddev-playwright

More random fails:

• https://git.drupalcode.org/project/ckeditor5_paste_filter/-/jobs/4490782
• https://git.drupalcode.org/project/ckeditor5_paste_filter/-/jobs/4490484

I found Drupal\editor\EditorXssFilter\Standard::filterXss() and was hoping that would solve this use case, but unfortunately it does not.

I'm reluctant to roll our own XSS filtering for this use case, in part because it still wouldn't be bulletproof. For example:

Search expression: <(em)>.*(bed).*<\/em>
Replacement: <$1$2>
Content: <em>go to bed</em>
Result: <embed>

I think we have two main options:

1. Keep XSS filtering and document this as a known limitation (we could also switch filtering to use Standard::filterXss())
2. Remove XSS filtering to allow for more use cases and acknowledge that there are many ways that security can go wrong when it comes to configuring text filters and formats beyond the scope of this module.

I need to examine the bigger picture more to see if there are other protections/mitigations that might allow us to justify removing XSS filtering the replacement text.

Closing out as I believe this is now outdated. Thanks!

Thanks for reporting this, after coming back to it I came up with what I think is a sensible way to test it.

star-szr → created an issue.

I have tested the MR on a real site, reviewed the code, and I think it's ready to go.

+1 to RTBC.

@prashant mishra Can you please update the issue credits to give us credit as well? Thanks in advance.

Add missing imports from converting to PHP attributes.

https://www.drupal.org/node/2825587/revisions/13798758/view →

Add missing drupal/rest dependency and switch code snippet to YAML format

I don't think the image quality in the MR is good enough, setting to needs work.

Pipeline that shows the test + fix and test-only jobs: https://git.drupalcode.org/issue/vite-3499492/-/pipelines/394927

star-szr → created an issue.

Tested by modifying composer.json repositories:

    "repositories": [
        {
            "type": "git",
            "url": "https://git.drupalcode.org/issue/cloudfront_cache_path_invalidate-3485969.git"
        },
        {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        }
    ],

And ran ddev composer require drupal/cloudfront_cache_path_invalidate:dev-3485969-incorrect-composer.json-psr-4

Result (no warnings!):

❯ ddev composer require drupal/cloudfront_cache_path_invalidate:dev-3485969-incorrect-composer.json-psr-4
./composer.json has been updated
Running composer update drupal/cloudfront_cache_path_invalidate
Loading composer repositories with package information
Updating dependencies
Lock file operations: 4 installs, 0 updates, 0 removals
  - Locking aws/aws-crt-php (v1.2.7)
  - Locking aws/aws-sdk-php (3.325.3)
  - Locking drupal/cloudfront_cache_path_invalidate (dev-3485969-incorrect-composer.json-psr-4 9f57dc8)
  - Locking mtdowling/jmespath.php (2.8.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 4 installs, 0 updates, 0 removals
  - Syncing drupal/cloudfront_cache_path_invalidate (dev-3485969-incorrect-composer.json-psr-4 9f57dc8) into cache
  - Installing aws/aws-crt-php (v1.2.7): Extracting archive
  - Installing mtdowling/jmespath.php (2.8.0): Extracting archive
  - Installing aws/aws-sdk-php (3.325.3): Extracting archive
  - Installing drupal/cloudfront_cache_path_invalidate (dev-3485969-incorrect-composer.json-psr-4 9f57dc8): Cloning 9f57dc89ed from cache
3 package suggestions were added by new dependencies, use `composer suggest` to see details.
Generating optimized autoload files
44 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Found 4 security vulnerability advisories affecting 3 packages.
Run "composer audit" for a full list of advisories.

star-szr → created an issue.

Thanks, nice idea. I'll see if I can cook up a test scenario that shows this bug.

I read through https://www.drupal.org/project/ckeditor_media_resize/issues/3471500#comm... 🐛 CKEditorError: plugincollection-plugin-not-found Needs work and that makes sense, I would still love a way to reproduce this though, so I can add automated testing around it. I'm still not clear on exactly when and how this bug would happen.

Thanks for the update! I definitely don't want to suggest you disable JS aggregation, just trying to diagnose this bug.

That library change might make sense, I think it's something I pulled from the ckeditor5_dev plugin starter template but maybe it's not a good idea. I did find https://www.drupal.org/project/ckeditor5_dev/issues/3401747 🐛 "plugin-not-found" when using the plugin starter template Closed: cannot reproduce which may be potentially related - same error message.

Thank you for mentioning paragraphs module, that might be an important piece. If you can provide more information about the structure of your content type that might point to a way to reproduce the bug.

Could you also try disabling JS aggregation and re-testing on your actual site (I'm assuming JS aggregation is enabled)? You may need to clear caches after.

Hi, thanks for the report.

After clearing caches do you see the same result?

What method are you using to install the module?

Can you be more specific about "trying to display a CKEditor field in the admin", is that something like editing a node with a body field, or is there custom code involved?

If you could write up steps to reproduce this from a clean install of Drupal that would be really helpful, since I'm not able to reproduce it in my testing. I tested with the same module and core versions (v1.0.1 on Drupal 10.3.5) but did not see any errorr, before or after clearing caches.

Thanks for the feature request @jstoller! I like the idea and don’t have any immediate concerns, although it will make the form vertically larger. I’m not sure when I can work on this feature in the near future but I’m happy to review patches/MRs and help it along with reviews and such.

star-szr → created an issue.

Closing because I don't think the bot has anything else to say, and 11.0.0 was released in early August.