Resaving menu links that points to a non-node entity changes the type to node and breaks the link

This looks like a fine workaround until we have the other issue solved.

Since displaying the autocomplete label for non-node entity types is a bug, we just have to update the test and use a node for that assertion :)

Beginning Triage

Confirmed that the bug is still present.
The patch no longer applies. Specifically the file in which the test that was modified lived has been moved and refactored, so that needs to be reworked. The line number in LinkWidget has changed.
The fix still works when applied by hand

I have re-rolled patch for the Drupal 8.4.x version.

@gaurav.kapoor please can you remember to include an interdiff where possible.

@Jo , i just re rolled #5. I don't think an interdiff was needed. Sorry for the confusion.

Remove line saving non-existent variable.
Replace deprecated variable.
(remove Needs Reroll tag)

My previous patch went wrong somehow, let's try again.

+++ b/core/modules/link/tests/src/Functional/LinkFieldTest.php
@@ -95,11 +96,7 @@ public function testURLValidation() {
-    // Create an entity with restricted view access.
-    $entity_test_no_label_access = EntityTest::create([
-      'name' => 'forbid_access',
-    ]);
-    $entity_test_no_label_access->save();
+    $restricted_node = $this->drupalCreateNode(['status' => NodeInterface::NOT_PUBLISHED]);

Here we are replacing a test with a custom entity with restricted access to a node with restricted access?

I think we need both tests. In addition, test an entity with unrestricted access, too.

I am adding credit to wolffereast for the triage work done. Just FYI, we like to see documented triage steps (even if brief). Here are some made-up examples of documented triage steps:

• I tested the steps to reproduce and they did (or did not) work (so I am tagging it "Needs issue summary update").
• I searched for duplicate issues but could not find any.
• I checked the issue summary and it is accurate and up-to-date.
• Etc...

Thank you!

Thank you, everyone, for working on this.

@xjm, @alexpott, @effulgentsia, @lauriii, @catch and I discussed this issue at a recent meeting and decided this is critical because it affects data integrity when people edit links.

(edited)

I believe this is a thoughtfull test including good description comments.

+++ b/core/modules/link/tests/src/Functional/LinkFieldTest.php
@@ -652,6 +649,67 @@ public function testLinkTypeOnLinkWidget() {
+    $entity_test_storage->resetCache();
+    $entity_test = $entity_test_storage->load($entity_test->id());

Note: This doesn't block a commit but you should be able to use loadUnchanged here instead, but seriously, these details matter less on tests, IMHO.

Committed a12adc7 and pushed to 8.4.x. Thanks!

This needs a re-roll on 8.3.x.

Re-rolled the patch from #22 for 8.3.x.

Re-roll looks fine to me. Let's see testbot's report.

Green!

Committed e5ee7c9 and pushed to 8.3.x. Thanks!

Thanks for the quick reroll.

Shouldn't we remove @todo now as patch has been submitted in https://www.drupal.org/node/2423093.