Comment subject link doesn't work in comment preview

I guess the only way to "solve" this is to show no link, if the cid is not available.

Update status

#6: comment_subject-655410-6.patch queued for re-testing.

+++ modules/comment/comment.module	3 May 2010 12:54:24 -0000
@@ -2172,10 +2172,15 @@ function template_preprocess_comment(&$v
+  }
+  ¶

trailing white space

Powered by Dreditor.

The patch still applies to latest dev, fixes the problem, and adheres to coding standards as I understand them. dereine do you agree this is RTBC?

Tests please.

I can still reproduce this using Drupal 7.2.

+++ modules/comment/comment.module	8 Oct 2010 12:20:27 -0000
@@ -2219,9 +2219,17 @@ function template_preprocess_comment(&$v
+  else {
+    $uri = array(
+      'path' => $_GET['q'],
+      'options' => array('attributes' => array(), 'fragment' => ' '),

This code should better be explained here.

In general the solution in #1048494: No title or wrong title in preview should be synced with this one, even it's not 100% sure which one is the better one.

Current Drupal 8.0.x-dev comment.module code creates the preview title link to front. Does this resolve this issue for D8?

  if (isset($comment->in_preview)) {
    $variables['title'] = \Drupal::l($comment->getSubject(), new Url('<front>'));
    $variables['permalink'] = \Drupal::l(t('Permalink'), new Url('<front>'));
  }
  else {
    $uri = $comment->urlInfo();
    $attributes = $uri->getOption('attributes') ?: array();
    $attributes += array('class' => array('permalink'), 'rel' => 'bookmark');
    $uri->setOption('attributes', $attributes);
    $variables['title'] = \Drupal::l($comment->getSubject(), $uri);

    $variables['permalink'] = \Drupal::l(t('Permalink'), $comment->permalink());
  }

I think it makes sense to provide a real link when comment not new (when preview called for edit comment form)
And yes this needs tests and total rewrite because code is changed

Why not make the link to #? i.e. remain in the preview

Good idea, comments in preview always have #new marker, so here's a new patch
Let's see the tests coverage

#24 looks great. Both links remain on the current page form the comment preview page.

I am updating the summary for better clarity.

Still needs tests

added test case.

please file a separete patch with test only to make sure that test fails without patch

patched with just the test alone.

Let's see

so "fail" is valid, please name the patch next time "patch_name-fail.patch"

Thanks for the test-only patch and the clear issue summary!

I thought we added something that made links in previews not take the user out of the preview a long time ago... I guess it was only the node preview. Since then the "real" preview functionality has improved it further and does this:

I agree with fixing this the "quick" way for now as it improves the usability slightly; linking to the front page is definitely wrong. In another issue though, we should explore using the same preview mechanism we have for nodes. Maybe for 8.1.x, though as a usability improvement it might even be acceptable for 8.0.x.

However, I don't think the approach in this patch is correct. When I preview a comment, the #new fragment ID does not even appear on the page, so they are just broken no-op links. It might as well be a link to #. Also, there are still other links on the page that can take the user out of the preview, like the username link, or any menu item for that matter since it's not considered an admin path the way that node preview is.

Maybe for an interim fix we should dig out the earlier patch for node preview links and use that fix -- I think it added some javascript or something to make all the links on the whole preview page not do anything? Failing that, I'd prefer to just link to # rather than a non-existent fragment ID. But let's at least try to find the previous issue and add it to the related issues.

1. +++ b/core/modules/comment/comment.module
   @@ -641,8 +641,11 @@ function template_preprocess_comment(&$variables) {
   +    // Link title and permalink to current page because comment could not be
   +    // saved now and we should not allow user to leave the page.
   

   Less important, but this comment confuses me a little. "Comment could not be saved now" doesn't seem correct. Maybe "Use the current page for the title and permalink in the preview, so that the user does not accidentally leave the page and lose work."

2. +++ b/core/modules/comment/src/Tests/CommentPreviewTest.php
   @@ -62,6 +62,9 @@ function testCommentPreview() {
   +    // Check that the permalink and subject link has #new fragment
   

   Minor: missing period at the end of the comment.

Rerolled and refactored, we are changing two links, so expanded the test coverage to cover both.

Since @xjm pointed out that this wasn't the best way of fixing this, not sure how valid this still is.

I have successfully applied patch #47 for drupal 9.4.x-dev. thanks @Lendude

Holy cow 13 years...

Testing on Drupal10.1 I can confirm the issue is still there. The subject link goes to the homepage which just doesn't make sense.

Applying the patch from #47 the link is now #new which is MUCH better.
Triggering D10 tests

If those pass I'll mark it.

Seems to have test failures that are valid.