🇩🇪Germany @mrshowerman

+1 for this feature!

We've been using a patch from MR 154 for many months now, and since then we did not experience the mentioned issue any more.
Patches from it didn't apply on latest version 8.x-1.40, so I rebased the branch.

mrshowerman → created an issue.

mrshowerman → created an issue.

This issue has been caused by a change to the Claro theme: 🐛 Prefix/Suffix not inline with autocomplete field Active
There's an open core issue at 🐛 Link field description squashes the field Active , so I guess this one should be closed as a duplicate.

Thanks for the feedback, @mark_fullmer.

1. That's why the new feature is opt-in. Of course, using the the aforementioned tools to spot and fix broken links is the desired behavior.
   But in cases where this can't be done (due to missing resources or whatever reason), this feature can improve UX for end users.
2. I like this idea! Adding a log entry for broken internal links can definitely improve the situation (and partly replace external QA tools).
3. Funny, that's exactly how we are styling such links in sites that make use of this feature:

   We're also adding a few other styles:

   .linkit-unavailable {
     color: var(--#{$prefix}link-color);
     text-decoration: underline wavy;
     cursor: help;
   }
   

   (The link color is Bootstrap specific, but I'm sure you get the idea)

mrshowerman → made their first commit to this issue’s fork.

It should me mentioned that for the text format filter, the correct order might be important.

If the Limit allowed HTML tags and correct faulty HTML filter is enabled and doesn't allow the <span> element, it should be placed above the Linkit filter, otherwise that element would be removed by it.

Thanks a lot for the review, @mark_fullmer.

The first attempt did perform an access check, so super users did see the link, as reported in #6.
However, I changed this behavior for the reasons mentioned in #15 and for another reason: there might be setups where anonymous users should be able to click on a link to a restricted page that they can view after logging in. That's why I replaced the "view access" check with a check on the status flag.

Link items should have those options, provided that Linkit's widget has been used in the node entity form.
Maybe this was not the case in your scenario?

Your suggested helper method is pretty similar to the existing helper method LinkitHelper::getEntityFromUri($uri), but I have to say that the whole concept of this feature is about the existence of the UUID data attribute, which provides proof that a node had been found by the LinkitWidget at the time the node had been edited.

@schillerm, I can't reproduce your issue. It is working for link fields as well as links in text fields in my setup.
The method isEntityLoadable() simply checks whether the appropriate data attributes (type and UUID) are present, so the call in line 124 should not be negated.
I renamed it to hasEntityAttributes() to better reflect this behavior and avoid misunderstandings.

Realized that doing an access check will lock out users that might be able to view an entity after login.
Now checking the publish status instead.

mrshowerman → created an issue.

This sounds exactly like the same issue I described in #3534699-14: Refactor plugin for CKEditor5 v45+ → .

@mark_fullmer, see my comment in #3534699-14: Refactor plugin for CKEditor5 v45+ → for another bug that is related to CKEditor 45. Not sure if this is the right place to report, but as @jannakha points out in their reply, it likely has to be fixed in Linkit.

Of course, I can also open a separate issue if that is preferred.

Using the patch from MR37 together with Linkit 7.0.8, which added a similar change, I am experiencing some strange behavior.

I have a link like this:

<a href="/node/30" data-entity-type="node" data-entity-uuid="85f7822b-f889-4515-9e20-1addb7ef8e57" data-entity-substitution="canonical">FOOBAR</a>

When I click on it and edit an attribute (e.g. "Open in new window"), the link is temporarily split up into 3 links: one for the part to the left of the cursor, one invisible filler link (containing only 7 ⁠ characters), and one for the part to the right of the cursor.

<a class="ck-link_selected" href="/node/30" data-entity-type="node" data-entity-uuid="85f7822b-f889-4515-9e20-1addb7ef8e57" data-entity-substitution="canonical" target="_blank">FOO</a>
<a class="ck-link_selected" href="/node/30" target="_blank">⁠⁠⁠⁠⁠⁠⁠</a>
<a class="ck-link_selected" href="/node/30" data-entity-type="node" data-entity-uuid="85f7822b-f889-4515-9e20-1addb7ef8e57" data-entity-substitution="canonical" target="_blank">BAR</a>

When the link dialog is closed, the 3 links are replaced with one link again.

<a href="/node/30" data-entity-type="node" data-entity-uuid="85f7822b-f889-4515-9e20-1addb7ef8e57" data-entity-substitution="canonical" target="_blank">FOOBAR</a>

@jannakha, do you have any idea why this happens?

While it normally shouldn't be a problem for most users, it messes up the display for us, because we add an icon to the text through CSS to links that open a new window.
Thus, the text temporarily looks like this:

FOO ↗ ↗BAR ↗
(should be FOOBAR ↗, of course)

Left a few comments on the MR.

This introduced a backwards incompatible change in the MigrateExecutable class constructor.

Now the following

$executable = new MigrateExecutable($migration, new MigrateMessage(), ['limit' => 10]);

fails with this error:

TypeError: Drupal\migrate_tools\MigrateExecutable::__construct(): Argument #3 ($keyValue) must be of type Drupal\Core\KeyValueStore\KeyValueFactoryInterface, array given

I like the idea with the filter setting.
For getting this to work with custom substitution plugins, we shouldn't hard-code the list of available options to canonical and media, and instead fetch all available media substitution plugins.
I added a commit to the MR that does this.

mrshowerman → created an issue.

Created an MR which adds a permission to bypass the access check.

There was a similar issue in Gin when editing paragraphs: 🐛 Paragraphs stick out from the edge Active
It is caused by CKEditor items in table cells that don't have an explicit width.

I created an MR that applies flexbox to the table rows and calculates the optimal width for the cells possibly containing the CKEditors.

mrshowerman → made their first commit to this issue’s fork.

This has been used for a week in production now, and we didn't notice any issues.
So RTBC+1 from my side.

mrshowerman → created an issue.

Earlier today, I updated the patch we use in one of our projects to the latest version of the MR.
Looks good at first sight, but I'd like to watch it a bit longer until giving feedback here.

I simply removed the maxlength attribute from both form fields.
I don't think it makes sense to increase the value to something random that might still be to small in some cases. There is no technical reason to limit the length.

The organization part is now reset before we pass the address to the formatter.

mrshowerman → changed the visibility of the branch 3521673-address-links-remove to hidden.

Nonsense. Issues shold always be targeted against the most current branch and cherry-picked to previous ones if necessary.

I'll take a look at this.

Not sure if this is a bug or a feature request, thus assigning to the 2.4.x-dev branch.

I have the same issue. Patch #2 fixes it. RTBC+1.

Tests are green again.

mrshowerman → changed the visibility of the branch 3048458-preload-attribute to active.

You have to tick the checkboxes below "Credit & committing" if you want to give credit 😉

Wow. No credit at all?

Wow. No credit at all?

Not sure which changes you're referring to, but I rebased anyway.

I've decided to create an MR in this case so we can better track updates to the branch.

There is no update script that enables the new module so far; I leave the decision of whether to add one to the maintainers.

mrshowerman → created an issue.

Attached patch fixes both issues.

mrshowerman → created an issue.

Rebased an addressed the @larowlan's comment.
Test fail seems unrelated.

Fixed phpcs, back to previous state.

mrshowerman → created an issue.

Rebased.

I noticed that in the HTML method, there were a few attributes added to the link that aren't valid HTML, like type="mailto". On the other hand, other valid attributes got lost in the pre and post keys that were not processed properly.

Created an MR that fixes both on top of the original patch.

LGTM.

+1 for this change.

There is a typo in the settings form description (missing comma), and the text could be optimised a little:

If checked, a button to rotate the image will appear next to each image.

Re #66: I agree that the 2 checkboxes should be moved, but since it concerns not only the new "Plays inline" box which this issue is about, but also the already existing "Muted", I'd vote for creating a separate issue for this to avoid scope creep.

We've been using this patch for a long time in various projects, and from my point of view, it is good to go.

Did a rebase. Now there's a few phpstan errors again, maybe @mohit_aghera can look into them.

Thanks a lot for the quick fix 🙏🏻

mrshowerman → created an issue.

Thanks for adding the target branch!
Rebased.

I rebased the MR.
@swentel, I added an example to the IS.

This should be targeted against 2.4.x-dev, but that's not yet available in the dropdown.

mrshowerman → created an issue.

Here's a fix.

mrshowerman → created an issue.

Re-roll.

Re-roll.

Rebased the MR and created a CR draft.
Also updated the deprecation target to 11.2.0 and the removal target to 12.0.0.
Please review.

Not sure if we need a followup for the removal of the deprecated interface.

Thanks for merging this, @finex!
But shouldn't this issue be fixed then?

Thanks for the feedback, @anybody. I applied your suggested changes.

Nice! Thanks for all the work, @mdranove. The new MR fixes my issue.
Having a closer look at the changes, I found a few things that could be improved, and took the liberty of changing them myself.

I'm also attaching a static patch that can be applied against 6.0.6 in case anyone needs it.

This looks good to go for me, thus setting in RTBC.

MR no longer applies, but it looks like the change from this issue has been included in 🐛 Create revision before changing values, set revision author and time Active , so this might be closed as a duplicate.

mrshowerman → created an issue.

mrshowerman → created an issue.

I was able to further reduce duplicate code by moving some parts of the viewElements() method into new trait methods. The slight differences between SvgImageFormatter and SvgResponsiveImageFormatter we noticed in 📌 Formatters shouldn't repeat core code Needs review today were respected, especially the cache tags handling 😉

mrshowerman → made their first commit to this issue’s fork.

Created a new MR that addresses all open issues.
Not sure what the right status should be now 🙈

Closing this as it was decided to work on this in the original issue.

@mrshowerman not sure we need to create a new issue for that, just open a new MR here with the fix.

Oops, missed that. I created 🐛 Fatal error when linking SVGs to file Active in the meantime, but I'll be happy to add the fix to this issue if that is preferred.

mrshowerman → created an issue.

@mably, I will also have a look into ✨ Use a trait to reduce code duplication in formatters Active .

Is there an open issue for this already?

Not yet. I had a quick look at this yesterday and was able to reproduce. I also have a fix ready.
Will create a new issue with that fix soon.

Addressed feedback by @grevil.

I made a few tests on several of our sites using different versions of Drupal (9.5.11, 10.2.0, 10.2.5, 10.2.7, and 10.3.6). All of them show the same error message when saving an unpublished node that has a URL alias. So I can confirm that 🐛 hook_node_grants implementations lead to a 'URL Alias' validation error when saving translated nodes. Fixed did not fix the issue.

I noticed that there is a second workaround: granting the "view any unpublished content" (provided by the module) also fixes the issue.

Updated the IS accordingly.

All feedback has been addressed, except for one thing where @mably requests input from @Grevil. Setting back to NR.