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.
Updating issue credits.
Done for this round.
Just seeing if i can update issue credits, please ignore.
star-szr → created an issue.
star-szr → created an issue.
As mentioned I think would need some additional logic since we can't assume there is a node_modules in the module/project directory, but I have something working with:
eslint-plugin-drupal-contrib
eslint-config-drupal
- .gitlab-ci.yml (same as above)
- Job (I'm not seeing the YAML errors locally, but anyway I'm going to be switching to eslint-plugin-drupal-contrib for now)
For what it's worth, I am looking into switching to https://www.npmjs.com/package/eslint-plugin-drupal-contrib and it looks like a nice improvement for my use case. When trying to use it I ran into a different but similar error message:
ESLint couldn't find the plugin "eslint-plugin-drupal-contrib".
(The package "eslint-plugin-drupal-contrib" was not found when loaded as a Node module from the directory "/builds/project/ckeditor5_paste_filter/web/core".)
It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
npm install eslint-plugin-drupal-contrib@latest --save-dev
The plugin "eslint-plugin-drupal-contrib" was referenced from the config file in ".eslintrc.json".
https://git.drupalcode.org/project/ckeditor5_paste_filter/-/jobs/1986803
star-szr → created an issue.
Thanks @jonathan1055! If I run `yarn install` from https://git.drupalcode.org/project/ckeditor5_paste_filter then I do get a node_modules/.bin
with eslint.
I am mostly guessing here but wondering if we should have logic to check for node_modules/.bin/eslint inside the module/project and prefer to use that eslint (presumably it will have an easier time resolving dependencies within its own node_modules like eslint-plugin-jsx-a11y), falling back to the core eslint which should be fine for modules/projects that don't bring in a specific eslint via package.json.
I'm going to close this for now. Not to say "never", but to say "not right now". Thank you for the feature request!
Great, thanks for following up!
Specifically, and I'm not sure the quickest way to test this myself, but something like:
$ $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml --format=junit --output-file=$CI_PROJECT_DIR/junit.xml $_ESLINT_EXTRA . || EXIT_CODE_FILE=$?
Thanks for following up! I see what you mean, the current working directory in that CI job is indeed the module (what I'm calling project in the Drupal.org sense because it could be a theme, or other) directory.
I was referring to this command:
$ $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml --format=junit --output-file=$CI_PROJECT_DIR/junit.xml $_ESLINT_EXTRA . || EXIT_CODE_FILE=$?
What I was suggesting is: Although it would only work with the correct dependencies in place in the module's directory, would running eslint from the node_modules of the module help?
Having said that, to get this working with eslint-config-drupal, might it just be a matter of running eslint from the project directory rather than core?
Reviewing other comments here, maybe what I should do is adopt https://www.npmjs.com/package/eslint-plugin-drupal-contrib and we should adopt others to do the same?
I just want to mention that https://www.npmjs.com/package/eslint-config-drupal hasn't been updated since October 15, 2020 but is still recommended in official documentation: https://www.drupal.org/node/1955232 →
https://github.com/theodoreb/eslint-config-drupal/issues/6 seems relevant, because eslint-config-drupal is bringing in dependencies that aren't in core (such as eslint-plugin-jsx-a11y).
Thanks for the report and for using the issue template.
I can’t easily test this at the moment but I suspect it’s essentially the same issue as 🐛 Pasting from Google Docs doesn't preserve some formatting Closed: won't fix , please see my first comment there.
The short version is that when you copy the rich text, strong and em are represented as span tags with inline styles. If you don’t remove inline styles ((<[^>]*) (style="[^"]*")
expression), then CKEditor 5 converts these to strong and em based on the inline styles matching certain criteria. Since that expression and its replacement does remove inline styles, CKEditor 5 doesn’t have the data it needs to determine the tag type based on the span.
I'd be happy with a closed status too :)
At the time it was just an idea, and I agree it's too late.
Thanks @bnjmnm!
Another possible approach would be to try to detect the source of the pasted HTML and only filter when certain sources (like MS Word or Google Docs) are detected.
I know CKEditor 5 itself uses logic like this for its paste from office plugin but it's also very easy to get bad markup from other sources, so I'm not totally sure if I want to implement this. Maybe as an opt-in feature with a warning that it only works for Microsoft Word and Google Docs. https://github.com/ckeditor/ckeditor5/blob/e71971206bb3e7e176692a1b973a4...
Hi @jacobupal! Thanks for writing this up.
I don't currently have any plans or desire to add this feature. Having a separate "paste from word" style button relies on people remembering it's there and using it. I think your idea of making more specific filter rules for your use case makes the most sense for your use case at this time.
Depending on the details of your use case, perhaps a global permission to allow users to skip filtering would be a simpler implementation. This has its own issues and complexities (administrators would get this permission by default) so I'm not fully sold on it either. It's just an idea at this point.
Some other workarounds to mention that should work with the current code, other than customizing rules or using multiple text formats:
- Drag and drop the content around inside the editor instead (admittedly cut/copy/paste is just better for some cases and perhaps more accessible, but I think it's worth mentioning)
- Move content around in the source editor instead
I forgot to reference this issue in my commit but: https://git.drupalcode.org/project/ckeditor5_paste_filter/-/commit/f1669...
Keeping this open in case the bot has any other suggestions.
This issue is specific to inline entity form (or at least forms-within-forms), it could potentially be incorporated into the linked issue at a later time, but I don't want to add to the confusion at this point.
Glad to hear it! Thanks.
Hi folks, I won’t be able to get back to this until sometime next week. If you want some data sooner feel free to open an MR against this project! From my understanding the existing MR can’t be turned into an issue fork MR but I might be wrong.
Hi, I think it might be a misunderstanding.
This module will filter rich text content pasted into the visual editor (WYSIWYG). If you paste plain text (HTML) into the visual editor, there's no rich text to filter.
One way to test and see what the module is doing would be to follow these steps:
- Visit https://developer.mozilla.org/en-US/docs/Web/HTML/Element/span#try_it
- Select and copy the content in the Output pane (where some words are highlighted in red)
- Navigate to your Drupal site where the CKEditor 5 instance is that you want to test, for example node add form
- Paste the content in your clipboard into the CKEditor 5 visual editor (WYSIWYG, not source editor)
If you follow these steps without this module installed (or with it disabled), the <span>
tags will be preserved. With this modules installed, enabled and with the default filters, the <span>
tags will be removed!
If you have another use case like filtering when you paste into the source editor, I'm not sure offhand what is possible there but that is different functionality.
I acknowledge this and will come back to this.
Attaching an updated patch that properly handles the field specificity (otherwise errors on all fields within the IEF will be skipped) and adds one defensive check.
Here: https://git.drupalcode.org/project/ckeditor5_paste_filter/-/pipelines/12...
It looks like it still ran but I didn’t dig into it. Let me know if I can be of any further help.
Hi @jonathan1055 it looks like that’s been merged, would you still like me to test?
As for this issue in general, I am hoping to work on GitLab CI things within the next month or two.
I think the direction makes sense. I agree that the way it used to work might be called “overbearing” or similar. Thanks for all your work @dqd!
I was just debugging some of this code with a colleague and I have to say it seems the existing code seems to support the use case.
I don't want to stoke the flames here, but it just doesn't seem right to call this a feature request when there is so much code that appears to now be dead code (admittedly I have not tested every use case) thanks to the hastily-added return
statement in
🐛
URL validation of link field doesn't work
Fixed
https://git.drupalcode.org/project/conditional_fields/-/blob/f988abcf081...
Of course dependentValidate()
as well.
Thanks for sharing, using a custom CKEditor 5 plugin that executes before our paste filter plugin is a viable approach (or just your custom plugin if you don't need any other paste filtering, no judgment!).
In a perfect world I would love to use the DOM instead of regular expressions for all of our paste filtering, but maintaining the same level of customization we currently have with the UI seems nearly impossible, and it would also make this module significantly more complex.
For specific use cases like this, at this time I am recommending that folks create a custom CKEditor 5 plugin.
Minor, but could you please correct the capitalization of the link text:
CKeditor 5 Paste Filter
Should be:
CKEditor 5 Paste Filter
@nkraft just to be sure, are you testing HTML comments along with other content? If the editor contains only an HTML comment it will still be stripped because CKEditor 5 doesn’t see any content to render.
Marking as fixed as I believe I have addressed the support request and there have been no further questions or follow-up. Thanks!
Marking as fixed as I believe I have addressed the support request and there have been no further questions or follow-up. Thanks!
Marking as fixed as I believe I have addressed the support request and there have been no further questions or follow-up. Thanks!
@annoul4 I have an idea what is happening, but would like to confirm before trying to explain fully. Overall I believe the filtering is working as expected. I suspect CKEditor 5 is transforming your filtered markup in a way that you are not expecting.
Overall I suspect you will both want something different from this module for removing empty paragraphs. Perhaps https://www.drupal.org/project/emptyparagraphkiller → is worth considering, or something similar could be implemented as a custom CKEditor 5 plugin.
I’m going to need more information. Please fill out the issue summary template so I can try to help you! You can see it when you create a new issue.
One possibility to keep in mind is that CKEditor 5 might be inserting these empty paragraphs after pasting.
1.0.0 is now out: https://www.drupal.org/project/ckeditor5_paste_filter/releases/1.0.0 →
Thank you for the detailed bug report and for making use of the issue summary template!
I was able to reproduce this behaviour and the root cause is how Google Docs sends its content to the pasteboard and I don't think much can be done about it within the scope of this module. You may be able to get it to work as you expect with some creative custom filters to preserve more of the formatting based on the original markup coming out of Google Docs, or by creating a custom CKEditor 5 plugin.
In short, our module and plugin is working as expected, but CKEditor 5 has its own magic that makes it seem like it should work differently. I'll break down the details below.
Google docs uses <span>
tags to convey all its formatting rather than semantic tags (<strong>
/<b>
or <em>
<i>
) as we might expect.
If we leave the span tags and attributes as-is by not filtering them out (your without filtering example), CKEditor 5 sees the span tags with specific style attributes from Google docs and turns them into the corresponding semantic tags. For example, font-style: italic
should be converted to <em>
. Since our default style removal filter removes all the style attributes before this transformation process, CKEditor 5 no longer has the data it needs to create the semantic tags and therefore does not preserve the formatting.
The example I'm testing with uses the following text in a Google doc:
This is a new paragraph that has bold and italic text.
Below is the markup that can be seen at the ClipboardPipeline#inputTransformation
event. This is the same event that our module acts on to do its filtering of the pasted content. The markup at this stage can be seen below and has been run through prettier
to make it a bit easier to read.
<p
style="line-height: 1.38; margin-bottom: 0pt; margin-top: 0pt"
dir="ltr"
id="docs-internal-guid-21dbe10f-7fff-36cd-034d-5b586bd69b9f"
>
<span
style="
background-color: transparent;
color: #000000;
font-family: Arial, sans-serif;
font-size: 11pt;
font-style: normal;
font-variant: normal;
font-weight: 400;
text-decoration: none;
vertical-align: baseline;
white-space: pre-wrap;
"
>This is a new paragraph that has </span
><span
style="
background-color: transparent;
color: #000000;
font-family: Arial, sans-serif;
font-size: 11pt;
font-style: normal;
font-variant: normal;
font-weight: 700;
text-decoration: none;
vertical-align: baseline;
white-space: pre-wrap;
"
>bold</span
><span
style="
background-color: transparent;
color: #000000;
font-family: Arial, sans-serif;
font-size: 11pt;
font-style: normal;
font-variant: normal;
font-weight: 400;
text-decoration: none;
vertical-align: baseline;
white-space: pre-wrap;
"
>
and </span
><span
style="
background-color: transparent;
color: #000000;
font-family: Arial, sans-serif;
font-size: 11pt;
font-style: italic;
font-variant: normal;
font-weight: 400;
text-decoration: none;
vertical-align: baseline;
white-space: pre-wrap;
"
>italic</span
><span
style="
background-color: transparent;
color: #000000;
font-family: Arial, sans-serif;
font-size: 11pt;
font-style: normal;
font-variant: normal;
font-weight: 400;
text-decoration: none;
vertical-align: baseline;
white-space: pre-wrap;
"
>
text.</span
>
</p>
(Hiding patches from the issue summary)
Core has a job that ensures compiled JS has been recompiled if needed: https://git.drupalcode.org/project/drupal/-/blob/02f6f6c68810136d707492c...
I decided to do both. I released 1.0.0-rc1 today and will plan to release 1.0.0 in a week or so if no critical issues are found in the release candidate.
Keeping this open until 1.0.0 is out.
This seems like a reasonable request. There have been no recent bug reports and the usage numbers keep steadily increasing so it does seem like a good time to cut a stable release.
We also don't have an API or upgrade path to consider: https://www.drupal.org/docs/develop/git/git-for-drupal-project-maintaine... →
I plan to cut a new release in the next week or so, either an rc or stable.
Thanks @Wim Leers!
Re: Nightwatch, I can sometimes trigger the failure locally, both with these changes and on 11.x. So it does seem unreliable/random.
This test specifically: core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroSearchFormTest.js
Same failure as last time, in Tests/Olivero/oliveroSearchFormTest. https://git.drupalcode.org/issue/drupal-3342874/-/jobs/288500#L174
If we remove the Nightwatch change it should be failing every time, in other words we need that change. I'm going to try running Nightwatch again.
I was seeing a failure in TimestampFormatterWithTimeDiffTest and one other similar one, seems like a random fail, re-ran and it passed. I re-ran Nightwatch and the failure switched from one to another so also seems random.
Now with more tests!
Hi @Wim Leers! This year I have been leading the project on our team to integrate CKEditor 5 which has involved porting and updating a contrib module → , adding table footer support to CKEditor 5 upstream, a few custom CKEditor plugins, and now this issue! So I suppose it was inevitable we would run into each other and it’s always nice to see a friendly face in the queues (and on GitHub)!
I will work on the test coverage, and agree that it’s missing.
Oh and thanks for adding the related issue. I did that at one point but had to fight with this form a bit (hence why it says there are so many patches from me in Credit & committing, I believe).
I found that our core drupalHtmlEngine
plugin is stripping out the comments. Tracing it back this was originally added in
#3247246: Attribute value encoding not compatible with Xss::filter() →
. I've added HTML comment support to the drupalHtmlEngine
plugin. I'm not 100% sure about the approach, feedback please!
I opened an MR to make code review easier and I'm attaching a patch with the same changes.
Testing note: If you test by adding only an HTML comment to the editor with no other content, it will still be removed. You need to add something else: an HTML element or text for example. This seems to be the case for the upstream plugin as well so I don't think that bug is on our side: https://ckeditor.com/docs/ckeditor5/latest/features/html/html-comments.html
Issue management note: I'm removing 'upstream' from the title because I think we have what we need to implement this but I do realize there are known issues with the upstream plugin and it's experimental. Maybe we can get something into core and then open another issue to track the known upstream issues?
star-szr → made their first commit to this issue’s fork.
Shout out to @Wim Leers who is doing some fantastic work in GitLab CI land.
This looks nice for the please fail if phpcs fails part: https://git.drupalcode.org/project/big_pipe_sessionless/-/blob/02e2d957d...
And this does not look that nice yet, but with some more work → I think it will be much cleaner and the testing setup closely aligns with ours (Nightwatch only): https://git.drupalcode.org/project/decoupled_pages/-/blob/9dfe432d560bff...
I did not know to check for that! I will run another test using https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/79
A couple updates to share here.
This looks nice for testing multiple core versions, but may or may not mesh with my plans around Nightwatch testing with a newer Nightwatch version:
#3397129: Allow modules to opt in to testing against Drupal previous major, previous minor, next minor →
Example implementation: https://git.drupalcode.org/project/big_pipe_sessionless/-/commit/0d37437...
It looks like PHPStan support is built-in now and would be trivial to add: #3395419: Allow contrib modules to run PHPStan via GitLab CI →
I can't speak as much to the code review aspect (though it does look good to me), but this works great, so marking as RTBC!
Ah, nevermind it looks like maybe you merged in 1.0.x or something? Sorry about that, I will leave it alone 😬
Here is that issue: #3398680: Incorrect instructions for testing issue forks in README →
star-szr → created an issue.
Thank you @Wim Leers and everyone. Catching up here and I can confirm that these changes fix the issue I reported here originally: https://git.drupalcode.org/project/ckeditor5_paste_filter/-/jobs/265709
_GITLAB_TEMPLATES_REF
= 3375359-nightwatch
_GITLAB_TEMPLATES_REPO
= issue/gitlab_templates-3375359
I tested by updating the CI variables on GitLab, which required a bit of digging and I think the README for this project needs a minor touch-up. I will be creating a small issue and MR for that.
Attaching a patch of the latest merge request against 4.2.5.
Hi, as noted in the Drupal.org interface text please don’t add random tags.
In addition, please consider using our issue template instead of deleting all of that structure and freestyling when reporting issues.
As for your request, just make use of regular expression features! See below.
Search expression:
<(button|canvas|caption)[^>]*>
Hi, I looked quickly and noticed that on regex101 there are different flags set. The help in the UI of this module indicates that we use the gimsu
flags.
Once I updated the flags on regex101 to match, their UI showed that there was an unnecessary escaping of a quote and removing the backslash made the replacement work again.
Untested, but try this version: https://regex101.com/r/yUjT7w/1
Regex: (<[^>]*) (data-(\S+)="((?:\\.|[^"\\])*)")
If the footnote linking to 2 has a href attribute of “sdendnote2sym” and so on, then it should be possible with a filter to add back the missing number.
If not, it may not be feasible since based on your description Word is actually removing the numbers. When “Filter Pasted Content” is unchecked this module isn’t doing anything to the pasted content.
Untested, but you can try something like the following.
Search expression:
<a href="#sdendnote([0-9]+)sym"> <\/a>
Replacement:
<a href="#sdendnote$1sym">$1</a>