- Issue created by @andy inman
- Merge request !3issue #3513457: Added support for the Paragraphs Edit module → (Merged) created by hhvardan
- 🇦🇲Armenia hhvardan
I’ve implemented a contextual link for paragraphs that allows copying to the clipboard via an AJAX callback.
The link is injected conditionally when the Paragraphs Edit → module is enabled. - 🇬🇧United Kingdom andy inman Gloucestershire, UK
Hi @Vardan Harutyunyan I’ve tested it with Drupal 10 using the https://dev-lcmd10.pantheonsite.io/ site.
Using copy-to-clipboard from the context menu works great. I tried pasting to into the same paragraph and into a different paragraph on a new node. Both ok. Check Mark
But I think there’s a potential security issue: the new route at /paragraphs-clipboard/paragraphs/{paragraph}/copy is accessible to users who would not have access to view the paragraph being copied. So maybe instead of using permission
access content
, you need to do an access check on the paragraph. It seems that Paragraphs Edit does something like that - I haven’t looked at the code, but looked at the related URLs from the context menu, and tried accessing them as anonymous user. I’m using this test page: https://lcmd10.lndo.site/en/node/20Edit: https://dev-lcmd10.pantheonsite.io/en/paragraphs_edit/node/20/paragraphs... → Access Denied. Check Mark
Clone: https://lcmd10.lndo.site/en/paragraphs_edit/node/20/paragraphs/4/clone?d... → Access Denied. Check Mark
Delete: https://lcmd10.lndo.site/en/paragraphs_edit/node/20/paragraphs/4/delete?... → Access Denied. Check Mark
Copy to clipboard: https://lcmd10.lndo.site/en/paragraphs-clipboard/paragraphs/4/copy?desti... → OK, returns JSON. Cross Mark
The JSON returned doesn’t seem to include any sensitive, data, but nonetheless, this behaviour doesn’t seem appropriate.
Finally, just a minor point: The Paragraph has been copied to clipboard message appears right at the top of the page, which is often not visible. It’s definitely useful to show a confirmation that the copy happened - maybe use a JS alert box instead? Or, insert at the top of the paragraph that was copied, but I imagine that would be a lot more work.
- 🇦🇲Armenia hhvardan
@Andy Inman Thanks for checking and for your helpful comment.
I updated the route to use _entity_access: 'paragraph.update' instead of 'access content'.
Now, only users who can edit the paragraph will be able to use the copy feature.About the message: you’re right that it’s easy to miss at the top of the page, but for now, I’ll keep using the default Drupal message system.
Showing the message closer to the paragraph would be a great improvement.
Using a JS alert could work too, but it’s not the best choice for Drupal UX and accessibility. - 🇬🇧United Kingdom andy inman Gloucestershire, UK
Hi @Vardan Harutyunyan I’ve retested, as before, and confirmed that access to the AJAX callback is now suitably restricted.
About the confirmation message - I agree it makes sense to keep it simple and standard. I did some reading: https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Ajax!MessageComma... - the part // A status message added an alternate location. part would be relevant - but would need a suitable location in the page. I notice that, when you click copy-to-clipboard, the Drupal AJAX spinner appears for a moment, inside the paragraph that was copied - that’s where the message could be shown. But I have no idea how you would identify that element. So, I’m sure it would be possible, but too much work for a small benefit.
-
hhvardan →
committed 93ee0e4e on 1.0.x
issue #3513457: Added support for the Paragraphs Edit module
-
hhvardan →
committed 93ee0e4e on 1.0.x