Submitting a comment in the non default language redirects you to the default language

I can confirm this bug.

I can confirm this bug too.

Adding the Issue Template to the description.

I was able to confirm the bug.

I'm marking this issue as 'needs work' with the caveat that the patch may be playing the role it needs to play. Pretty complex matrix of possibilities. Of course it is - it’s multilingual.

Unfortunately (and admittedly with no expert insights into how all the contributing chunks need to link up) I’m not getting the expected results when I save a comment with the patch applied. After the patch, the url to which users are redirected on saving a comment does not contain the language code (uncertain if in fact it should, but I believe it should). The interface has an unexpected mix of languages, perhaps indicating that the patch is working in part. If I enter the language code in the url, I see the unadulterated interface language I anticipate.

My steps to reproduce the issue are in the issue summary; the steps include a lot of detail in case variation is needed to ensure consistency of outcome. It’s certainly worth reviewing my steps in case something there is affecting the context of the patch.

Patch applies cleanly.

Errors appear in Watchdog, esp.

| locale | 04/05/2018 - 22:30 | Created JavaScript translation file for the language… | Kay V |
| page not found | 04/05/2018 - 22:30 | /fr/node/3 | Kay V |
| page not found | 04/05/2018 - 22:30 | /en/node/2 | Kay V |
| locale | 04/05/2018 - 22:30 | Updated JavaScript translation file for the language… | Kay V |
| comment | 04/05/2018 - 22:30 | Comment posted: suppongo che il prossimo…. | Kay V | Visualizza |

Notes on what wouldn't be possible to know from the watchdog lines above:

• they are pasted just as they appear in the watchdog UI at /en/admin/reports/dblog, meaning that the lowest one happened first in the chronology
• i.e. comment posted in Italian, the default language, then "page not found" in 2 of the 3 expected languages, but no mention of Chinese, simplified
• also no instances of "page not found" after subsequent posting of comments in French and English

I'm also affected by this.
More over, I can't even get the comments to display in the language they were submitted in
https://drupal.stackexchange.com/questions/257610/multilingual-comments-...

@alexseif, there are 2 others patches that you need to apply to get the comments fully functional when you have a multilingual site.

#2751267-16: Comments are not filtered on language
#2958935-9: Comments created in translation are displayed only for admin role

Does this issue (and related issues) possibly merit a higher level of urgency ("major" vs "normal")? It appears, from my brief glimpse at its scope while testing the proposed patch, that the effect on commenters is substantial, and the system affected (the comment system) is a fundamental part of Drupal core. Just floating the notion.

@kay_v yes there are many problems with the comments and the multilingual.

All related issues needs work still, in multilingual env looks filtering is best solution in long run

+++ b/core/modules/comment/src/CommentForm.php
@@ -357,7 +357,7 @@ public function preview(array &$form, FormStateInterface $form_state) {
-    $entity = $comment->getCommentedEntity();
+    $entity = $this->entityManager->getTranslationFromContext($comment->getCommentedEntity());

it should be entity repository service, so it needs injection

If i don't have content in current language (node opened in fallback language mode), then after submit comment i still redirecting to default node language. To prevent this behavior i used this patch

I'm also seeing this behaviour when deleting comments. That is, if I delete a French comment, I'm redirected to the English (default) page.

The patch in #20 works nicely when creating new comments, though.

Here's a patch that uses the same approach as #20, to fix redirects following deleting a comment too.

Patch ported for 8.7.x., where urlInfo() became toUrl(), preventing #23 from applying.

Helping patch #23 it's good to solve the above problem.

@ergonlogic thanks.

remove deprecated function.

#34 works great!

As bug it requires test

Executed the patch #34 against Drupal 8.9.x-dev

https://dispatcher.drupalci.org/job/drupal_patches/44839/

The patch at #34 worked for me on 8.9.x

patch at #34 worked as expected.
On adding comment in non default language it does not redirects to default language as expected and while adding comment in default language system remains in default language.

Tested on 8.9.x-dev, with English(en) as default language and Hindi(hi) being non-default language.

Moving to RTBC.

#DrupalIndiaAssociation

This looks an important fix for multi-lingual sites.

Thanks for filing this bug report and for fixing the issue. Bug fixing is very valuable. In order to commit a bug fix, we need an automated to test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal 8 see the following links:

1. https://www.drupal.org/docs/8/testing
2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/8.9.x

I've checked the comment tests and it appears that there is not a test of commenting on a translated entity. Adding a test that does that seems essential to proving we've fixed this issue.

Creating tests.

Added test for content and its translated counterpart.

Nice! Please file separately test-only.patch to prove it catch regression

Thanks for writing the test @rahul.shinde - it's great to see multilingual comments tested.

1. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   + * Tests Comment On Multilingual Content/
   

   Could be something like...

   Tests comments on multilingual content.</coode>
   </li>
   
   <li>
   <code>
   +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +  public static $modules = [
   

   protected

2. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  protected function setUp() {
   

   For D9 this needs to have void return type hint.

3. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +  /**
   +   * Adds additional languages.
   +   */
   +  protected function setupLanguages() {
   +    foreach (['mr', 'fr'] as $langcode) {
   +      ConfigurableLanguage::createFromLangcode($langcode)->save();
   +    }
   +  }
   +
   +  /**
   +   * Creates Article node type with comment field.
   +   */
   +  public function setupBundle() {
   +    // Create a article content type.
   +    $this->drupalCreateContentType(['type' => 'article', 'name' => t('Article')]);
   +    // Create comment field on article.
   +    $this->addDefaultCommentField('node', 'article');
   +  }
   

   I don't think that adding methods that is only called once from setUp() is needed here. They're not that long and adds the amount you need to think about when reviewing the test.

4. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +   * Post a comment on given node.
   

   Posts a comment on a given node.

   First line of function docs need to start with a verb that ends in an s... there are some exceptions see https://www.drupal.org/docs/develop/standards/api-documentation-and-comm...

5. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +    $subject = $this->randomMachineName();
   +    $comment = $this->randomMachineName();
   

   There's not need for these to be machine names. Something like \Drupal\Component\Utility\Random::sentences() is more appropriate.

6. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +    $patternPrefix = '';
   +    $pathPrefix = '';
   +
   +    // Here we are composing the prefixes for comment form and the pattern to
   +    // match the after post comment.
   +    if (!$language->isDefault()) {
   +      $pathPrefix = $node->language()->getId();
   +      $patternPrefix = $pathPrefix;
   +      $pathPrefix .= '/';
   +      $patternPrefix .= '\/';
   +    }
   ...
   +    $pattern = '/' . $patternPrefix;
   +    $pattern .= 'node\/([0-9]+)#comment-([0-9]+)';
   +    $pattern .= '/';
   +
   +    // Check the url.
   +    preg_match($pattern, $this->getURL(), $match);
   +
   +    $this->assertTrue(isset($match[1]) && isset($match[2]), 'There is issue with posting comment for ' . $language->getName() . '.');
   

   Let's assert the path outside of this method. After calling it. Then we don't need to do all of this work and what the test is testing is easier discern by reading it.

7. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +    // Determine the visibility of subject form field.
   +    $display_repository = $this->container->get('entity_display.repository');
   +    if ($display_repository->getFormDisplay('comment', 'comment')->getComponent('subject')) {
   +      // Subject input allowed.
   +      $edit['subject[0][value]'] = $subject;
   +    }
   +    else {
   +      $this->assertNoFieldByName('subject[0][value]', '', 'Subject field not found.');
   +    }
   

   I don't think this dynamic-ness is necessary. We only exercise one of these code paths in the test.

8. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +    $this->drupalPostForm(NULL, $edit, t('Save'));
   

   Let's assert a 200 response here.

9. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,188 @@
   +    $english_node = $this->reloadEntity($english_node);
   ...
   +    $marathi_node = $this->reloadEntity($english_node)->getTranslation('mr');
   ...
   +    $french_node = $this->reloadEntity($english_node)->getTranslation('fr');
   

   Why do we need the reload entity functionality? I'm not sure this is needed.

Updated test, added test-only patch.

1. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,144 @@
   +    // Add languages.
   +    foreach (['mr', 'fr'] as $lang) {
   +      ConfigurableLanguage::createFromLangcode($lang)->save();
   +    }
   

   After adding the languages we need to call $this->resetAll(); so the container is in multilingual mode.

2. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,144 @@
   +  protected function postCommentOnMultilingualContent(EntityInterface $node) {
   +
   +    $subject = $this->randomString(4);
   +    $comment = $this->getRandomGenerator()->paragraphs(2);
   +
   +    $language = $node->language();
   +
   +    $path_prefix = '';
   +
   +    // Here, we are composing the prefixes for comment form to post comment.
   +    if (!$language->isDefault()) {
   +      $path_prefix = $node->language()->getId();
   +      $path_prefix .= '/';
   +    }
   +
   +    // Get the comment form.
   +    $this->drupalGet($path_prefix . 'comment/reply/' . $node->getEntityTypeId() . '/' . $node->id() . '/comment');
   

   Here we should pass in $langcode and rewrite this to use the routing system. Less hardcoding of urls and constructing prefixes. So it can be something like

     protected function postCommentOnMultilingualContent(EntityInterface $node, $langcode) {
       $subject = $this->randomString(4);
       $comment = $this->getRandomGenerator()->paragraphs(2);
   
       $options = [
         'language' => ConfigurableLanguage::load($langcode);
       ];
   
       // Get the comment form.
       $url = Url::fromRoute('comment.reply', [
         'entity_type' => $node->getEntityTypeId(),
         'entity' => $node->id(),
         'field_name' => 'comment',
       ], $options);
       $this->drupalGet($url);
   

3. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,144 @@
   +    $subject = $this->randomString(4);
   +    $comment = $this->getRandomGenerator()->paragraphs(2);
   ...
   +    $edit['subject[0][value]'] = $subject;
   +    $edit['comment_body[0][value]'] = $comment;
   

   Rather than creating $subject and $comment variables - let's do something like:
   $edit['subject[0][value]'] = $this->randomString();

   Also not not using (4) because the shorter the string the potential more work to do.

4. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,144 @@
   +    $english_node = Node::create([
   +      'type' => 'article',
   +      'title' => 'Test title',
   +    ]);
   +
   +    $english_node
   +      ->setPublished()
   +      ->save();
   

   Let's call this $node. And then we can do something like...

       // Create multilingual content.
       $node = Node::create([
         'type' => 'article',
         'title' => 'Test title',
       ]);
       $node
         ->addTranslation('mr', ['title' => 'Marathi title'])
         ->addTranslation('fr', ['title' => 'French title'])
         ->save();
   

5. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,144 @@
   +    $marathi_node = $english_node->addTranslation('mr', ['title' => 'Marathi title']);
   +    $marathi_node
   +      ->setPublished()
   +      ->save();
   +
   +    // Add french translation.
   +    $french_node = $english_node->addTranslation('fr', ['title' => 'French title']);
   +    $french_node
   +      ->setPublished()
   +      ->save();
   

   We don't need to assign $marathi_node or $french_node here. See above...

6. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,144 @@
   +    // Post a comment on marathi translation.
   +    $marathi_node = $english_node->getTranslation('mr');
   +    $this->postCommentOnMultilingualContent($marathi_node);
   

   We the updated signature this would be something like:

       // Post a comment on marathi translation.
       $this->postCommentOnMultilingualContent($node, 'mr');
       $this->assertSession()->addressEquals('mr/node/1#comment-2');
   

   The $this->assertSession()->addressEquals('mr/node/1#comment-2'); looks really nice now. It's easy to determine what is being tested.

On it

@alexpott i please review this patch the changes has been done as suggested by you in #50.
RE #50.1:Done
RE #50.2:Done
RE #50.3:Done
RE #50.4:Done
RE #50.5:Done
RE #50.6:Done

Fix lint issue

Working on it need to create new patch failed to apply on 8.9

@pavnish thanks for working on this. It's a really good idea if you are submitting a patch that adds a new test (or changes a test) to run the test locally before you upload the patch to drupal.org.

Reviewing #51...

1. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,128 @@
   +  /**
   +   * Modules to install.
   +   *
   +   * @var array
   +   */
   +  protected static $modules = [
   

   Can be {@inheritdoc}

2. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,128 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  protected function setUp(): void {
   

   The :void here makes this only for D9.

3. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,128 @@
   +  protected function postCommentOnMultilingualContent(EntityInterface $node, $langcode) {
   

   Need to document the $langcode parameter.

4. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,128 @@
   +    $subject = $this->randomString(4);
   +    $comment = $this->getRandomGenerator()->paragraphs(2);
   

   This is not needed.

5. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,128 @@
   +    $options = [
   +      'language' => ConfigurableLanguage::load($langcode);
   +    ];
   

   Unexpected colon.

6. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,128 @@
   +    $edit['subject[0][value]'] = $this->randomString();
   +    $edit['comment_body[0][value]'] = $comment;
   

   We can set the comment without $comment too.

7. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,128 @@
   +    $this->postCommentOnMultilingualContent($node);
   

   Should pass the 'en' langcode too.

1. +++ b/core/modules/comment/src/CommentForm.php
   @@ -369,9 +369,9 @@ public function preview(array &$form, FormStateInterface $form_state) {
   -    $uri = $entity->toUrl();
   +    $uri = $entity->toUrl('canonical', ['language' => \Drupal::languageManager()->getCurrentLanguage()]);
   

   The test passes without this change. I'm not sure that this is necessary. Or maybe it is but we've not tested it.

2. +++ b/core/modules/comment/src/Form/DeleteForm.php
   @@ -16,7 +16,7 @@ class DeleteForm extends ContentEntityDeleteForm {
      public function getCancelUrl() {
        // Point to the entity of which this comment is a reply.
   -    return $this->entity->get('entity_id')->entity->toUrl();
   +    return $this->entity->get('entity_id')->entity->toUrl('canonical', ['language' => \Drupal::languageManager()->getCurrentLanguage()]);
      }
   

   We should add test coverage of deleting one of the comments we've added to. And ensure we end up on the correct url.

3. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,128 @@
   +    // Get the comment form.
   +    $url = Url::fromRoute('comment.reply', [
   

   Need to add use Drupal\Core\Url; to the use statements.

4. Also it seems when you comment on a French node the comment only end up on the english node!!!! So right now you're getting redirected back to the correct translated node but you can't see the comment you've just made.

@alexpott, I am uploading following addressing comment #50,

• interdiff_49_58.txt
• submmiting_comment_redirection_issue-2751269-58.patch
• test-only.patch

Thank you @alexpott being patient with reviewing changes and suggestions.

#57 still needs addressing.

1. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,136 @@
   +  /**
   +   * Modules to install.
   +   *
   +   * @var array
   +   */
   +  protected static $modules = [
   

   {@inheritdoc}

2. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,136 @@
   +   * @param \Drupal\Core\Entity\EntityInterface $node
   +   *   Node to post comment on.
   +   *
   +   * @param $langcode
   +   *   String language code..
   

   No empty line between the @param's

3. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,136 @@
   +    // Post a comment on default language.
   +    $this->postCommentOnMultilingualContent($node, $node->language()->getId());
   ...
   +    $langcode = 'mr';
   +    $marathi_node = $node->getTranslation($langcode);
   +    $this->postCommentOnMultilingualContent($marathi_node, $langcode);
   ...
   +    $langcode = 'fr';
   +    $french_node = $node->getTranslation($langcode);
   +    $this->postCommentOnMultilingualContent($french_node, $langcode);
   

   There's no need to get the translation here. And we can hardcode the langcode passed into postCommentOnMultilingualContent

Working on #59

@alexpott, I am uploading following addressing comment #59, #57 still need to work looking on test case
RE #59.1:Done
RE #59.2:Done
RE #59.3:Done
interdiff_58_61.txt
submmiting_comment_redirection_issue-2751269-61.patch
test-only.patch
Thank you @alexpott

+++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
@@ -0,0 +1,129 @@
+    $this->assertSession()->addressEquals('fr/node/1#comment-2');

This will fail as the path is wrong. It should be 'fr/node/1#comment-3'

@rahul.shinde thanks Please review new patch.It's due to the last change was not saved in my system so the patch was failed.
I was checked in my local by phpunit e.i working fine.i think CTRL+Z applied my mistake at the patch creation time.

@alexpott, I am uploading following addressing comment #59, #57 still need to work looking on test case
RE #59.1:Done
RE #59.2:Done
RE #59.3:Done
interdiff_58_65.txt
submmiting_comment_redirection_issue-2751269-65.patch
test-only.patch
Thank you @alexpott

Thanks for working on this, couple of questions/nits

1. +++ b/core/modules/comment/src/CommentForm.php
   @@ -369,9 +369,9 @@ public function preview(array &$form, FormStateInterface $form_state) {
   -    $uri = $entity->toUrl();
   +    $uri = $entity->toUrl('canonical', ['language' => \Drupal::languageManager()->getCurrentLanguage()]);
   

   Do we need this - I'm not sure we do - looking at the code in ->toUrl it is language aware already.

   see this hunk in Entity::toUrl()

   
   // Display links by default based on the current language.
       // Link relations that do not require an existing entity should not be
       // affected by this entity's language, however.
       if (!in_array($rel, ['collection', 'add-page', 'add-form'], TRUE)) {
         $options += ['language' => $this->language()];
       }
   

   If we do, then we should inject the language manager, as CommentForm already uses container injection.

2. +++ b/core/modules/comment/src/Form/DeleteForm.php
   @@ -16,7 +16,7 @@ class DeleteForm extends ContentEntityDeleteForm {
   -    return $this->entity->get('entity_id')->entity->toUrl();
   +    return $this->entity->get('entity_id')->entity->toUrl('canonical', ['language' => \Drupal::languageManager()->getCurrentLanguage()]);
   

   Is there a reason we don't use ->getTranslationFromContext here so the two are consistent?

   It would require injecting the entity repository, but this class already has container injection from its parent classes.

   We'd not need the language manager if we used the entity-repository approach.

3. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,129 @@
   +    // Create a article content type.
   

   ubernit: should be 'an' Article content type (I realise English may be your second language, so apologies for nitpick)

4. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,129 @@
   +   *   String language code..
   

   ubernit: two . here

5. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,129 @@
   +    $this->assertSession()->addressEquals('node/1#comment-1');
   ...
   +    $this->assertSession()->addressEquals('mr/node/1#comment-2');
   ...
   +    $this->assertSession()->addressEquals('fr/node/1#comment-3');
   

   ubernit, we can store $this->assertSession() in a local variable here to avoid creating three instances

The big issue that needs to be resolved here is

Also it seems when you comment on a French node the comment only end up on the english node!!!! So right now you're getting redirected back to the correct translated node but you can't see the comment you've just made.

So while the change makes sense its currently resulting in a much worse user-experience because you're being taken to a page without the comment you've just made!!!!

Based on the above reviews and running phpcs on various files.

a few more fixes from the previous patch.

I'm not sure it doable yet without solving other issues like
- #2301465: Make comment threading per entity translation
- #2751267: Comments are not filtered on language
- #2990248: Make Comment Admin Overview query alterable

1. +++ b/core/modules/comment/src/CommentForm.php
   @@ -369,9 +369,9 @@ public function preview(array &$form, FormStateInterface $form_state) {
   -    $uri = $entity->toUrl();
   +    $uri = $entity->toUrl('canonical');
   
   +++ b/core/modules/comment/src/Form/DeleteForm.php
   @@ -16,7 +16,7 @@ class DeleteForm extends ContentEntityDeleteForm {
   -    return $this->entity->get('entity_id')->entity->toUrl();
   +    return $this->entity->get('entity_id')->entity->toUrl('canonical');
   

   See \Drupal\Core\Entity\EntityInterface::toUrl()

   public function toUrl($rel = 'canonical', array $options = []);
   

   canonical is the default. So unnecessary changes.

2. +++ b/core/modules/comment/src/Tests/Views/CommentTestBase.php
   @@ -63,6 +63,9 @@ abstract class CommentTestBase extends ViewTestBase {
   +   * {@inheritDoc}
   
   +++ b/core/modules/comment/tests/src/Functional/CommentAdminTest.php
   @@ -20,6 +20,9 @@ class CommentAdminTest extends CommentTestBase {
   +   * {@inheritDoc}
   
   +++ b/core/modules/comment/tests/src/Functional/CommentAnonymousTest.php
   @@ -17,6 +17,9 @@ class CommentAnonymousTest extends CommentTestBase {
   +   * {@inheritDoc}
   
   +++ b/core/modules/comment/tests/src/Functional/CommentBlockTest.php
   @@ -24,6 +24,9 @@ class CommentBlockTest extends CommentTestBase {
   +   * {@inheritDoc}
   
   +++ b/core/modules/comment/tests/src/Functional/CommentBookTest.php
   @@ -29,6 +29,9 @@ class CommentBookTest extends BrowserTestBase {
   +   * {@inheritDoc}
   
   +++ b/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php
   @@ -62,6 +62,9 @@ class CommentTranslationUITest extends ContentTranslationUITestBase {
   +   * {@inheritDoc}
   
   +++ b/core/modules/comment/tests/src/Functional/Views/CommentFieldFilterTest.php
   @@ -36,6 +36,9 @@ class CommentFieldFilterTest extends CommentTestBase {
   +   * {@inheritDoc}
   
   +++ b/core/modules/comment/tests/src/Functional/Views/CommentRestExportTest.php
   @@ -35,6 +35,9 @@ class CommentRestExportTest extends CommentTestBase {
   +   * {@inheritDoc}
   
   +++ b/core/modules/comment/tests/src/Functional/Views/CommentTestBase.php
   @@ -56,6 +56,9 @@ abstract class CommentTestBase extends ViewTestBase {
   +   * {@inheritDoc}
   

   {@inheritdoc}, lowercase

3. +++ b/core/modules/comment/tests/src/Functional/CommentAdminTest.php
   @@ -40,7 +43,7 @@ public function testApprovalAdminInterface() {
   -    // Test that the comments page loads correctly when there are no comments
   +    // Test that the comments page loads correctly when there are no comments.
   

   Tests that ... . start with Tests

4. +++ b/core/modules/comment/tests/src/Functional/Views/DefaultViewRecentCommentsTest.php
   @@ -65,10 +65,13 @@ class DefaultViewRecentCommentsTest extends ViewTestBase {
   +  /**
   +   *
   +   */
      protected function setUp($import_test_views = TRUE) {
   

   {@inheritdoc} or unexpected change.

5. +++ b/core/modules/comment/tests/src/Functional/Views/WizardTest.php
   @@ -56,7 +56,6 @@ public function testCommentWizard() {
   -
   

   Or unexpected change, but it's ok.

A lot of unexpected changes here, for coding standards violations, we do not have to get all passed, just get which enabled rules/sniffs passed. I would suggest reverting all unexpected changes to coding standard violations. leave them to coding standards relevant issues.

One way to check coding standard violations against ENABLED rules/sniffs

composer run phpcs FILES/DIRS/PATHES/TO/CHECK

Checkout the composer.json for more:

        "phpcs": "phpcs --standard=core/phpcs.xml.dist --runtime-set installed_paths $($COMPOSER_BINARY config vendor-dir)/drupal/coder/coder_sniffer --",

Working On it

@jungle Hi could you please review this patch all the suggestion in #74 has been implemented.
Thanks
Pavnish

Thanks, @pavnish!

Reverting all unnecessary coding standard changes. Changing the target version to 9.1.x.

Continue working on it next.

1. See #67.5

   +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,129 @@
   +    $this->assertSession()->addressEquals('node/1#comment-1');
   ...
   +    $this->assertSession()->addressEquals('mr/node/1#comment-2');
   ...
   +    $this->assertSession()->addressEquals('fr/node/1#comment-3');
   

   ubernit, we can store $this->assertSession() in a local variable here to avoid creating three instances

2. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,129 @@
   +    $edit['subject[0][value]'] = $this->randomString();;
   

   Two ;

3. +++ b/core/modules/comment/tests/src/Functional/CommentLanguageTest.php
   @@ -129,9 +129,9 @@ public function testCommentLanguage() {
   +        $this->assertEqual($comment->get('langcode')->value, $langcode, new FormattableMarkup('The comment posted with content language %langcode and belonging to the node with language %node_language has language %comment_language', $args));
   +        $this->assertEqual($comment->get('comment_body')->value, $comment_values[$node_langcode][$langcode], 'Comment body correctly stored.');
   

   assertEquals()
   

4. +++ b/core/modules/comment/tests/src/Functional/CommentLanguageTest.php
   @@ -129,9 +129,9 @@ public function testCommentLanguage() {
   -        $this->assertEqual($comment->langcode->value, $langcode, new FormattableMarkup('The comment posted with content language %langcode and belonging to the node with language %node_language has language %comment_language', $args));
   -        $this->assertEqual($comment->comment_body->value, $comment_values[$node_langcode][$langcode], 'Comment body correctly stored.');
   ...
   +        $this->assertEqual($comment->get('langcode')->value, $langcode, new FormattableMarkup('The comment posted with content language %langcode and belonging to the node with language %node_language has language %comment_language', $args));
   +        $this->assertEqual($comment->get('comment_body')->value, $comment_values[$node_langcode][$langcode], 'Comment body correctly stored.');
   
   +++ b/core/modules/comment/tests/src/Functional/Views/CommentFieldFilterTest.php
   @@ -67,7 +67,7 @@ protected function setUp($import_test_views = TRUE) {
   -    $this->comment->langcode = 'en';
   +    $this->comment->set('langcode', 'en');
   

   Good with this kind of changes. as $this->comment->langcode, $comment->comment_body->value are accessing protected properties. maybe we need a follow up to clear up other occurrences cross the core.

Tagging "Needs followup" for #79.4. Addressing #79.1,2 and 3.

+++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
@@ -0,0 +1,131 @@
+  /**
+   * Tests redirection of comment posted on multilingual content.
+   *
+   * Here we tests redirection of the user to the node translation where the
+   * comment is made.
+   *
+   * @throws \Behat\Mink\Exception\ExpectationException
+   */
+  public function testCommentOnMultilingualContent() {

One line is enough here. Especially @throws here makes no sense.

Suggestion:

Tests redirections with posting comments on multilingual content.

I read through the issue checking that all the work is complete. I found these are still to do:

#67.4
#79.4
#81

The last manual test was done in #22 on the patch in #20. There has a been a change to CommentForm.php since then so this needs manual testing.

And one question from a brief look at the patch.

1. +++ b/core/modules/comment/tests/src/Functional/Views/CommentFieldFilterTest.php
   @@ -67,7 +67,7 @@ protected function setUp($import_test_views = TRUE): void {
   -    $this->comment->langcode = 'en';
   +    $this->comment->set('langcode', 'en');
   

   this looks like it is out of scope.

Addressing pointers from #82

Unrelated test failure

Patch#83 is working as expected-" if I add a comment on a node which is not in the default language, I get redirected to the same translation of the node." Tested it on Drupal 9 version. +1 for RTBC.
Screenshots are attached.

Steps:

1. Visit /admin/modules
2. Install "Language","Configuration Translation","Content Translation"," Interface Translation"
3. Visit /admin/config/regional/language
4. Add "French" language
5. Visit /admin/config/regional/content-language
4. Select Comment and Content from Custom language settings
5. Then Select "TRANSLATABLE" checkbox given against Comment type, Content type- Article and Basic page
6. Mark tick against the checkbox "Show language selector on create and edit pages" Comment type, Content type- Article and Basic page.
7. Save the settings.
8. Create an Article page in default language(English) and then create a french translation for same page.
9. Post comment with French language selection on the french translated page.
10. You will notice the link generated on the posting of comment, url will have "fr" code on it.
11. Now if a comment is posted with Default language(English) selection on french translated page.
12. You will notice the comment link will appear with no code on it.

Applied the patch and seems to be working as expected. Just added some ubernit stuff as per interdiff. Moving to RTBC.

#69 is still not addressed... repeating the comment because it is important.

The big issue that needs to be resolved here is

Also it seems when you comment on a French node the comment only end up on the english node!!!! So right now you're getting redirected back to the correct translated node but you can't see the comment you've just made.

So while the change makes sense its currently resulting in a much worse user-experience because you're being taken to a page without the comment you've just made!!!!

Also there's #72 from @andypost who is a maintainer of the comment module.

Can somebody please give a responsible answer to the following question:

If I make a patch to the kernel to address the above issues and build a production website, if in the future the core is corrected and fixes the above will I be able to update the core and remove the patch, or the comments that will have been created with the applied patch will not be compatible with a future corrected core?

I'm building a new multilingual D9 site the last weeks and I selected Drupal over wordpress because I considered Drupal more mature regarding multilingual capabilities, but the moment I realized the problem with the comments all my plan collapsed and now I don't know what to do. The above problems are critical and I don't want to patch the core.

Can we address the redirect issues in a custom module? What do you suggest to do in order to continue building the site on D9 and to not surrender to wordpress?

In the patch I see changes in the following files:
/core/modules/comment/src/CommentForm.php ->patched
/core/modules/comment/tests/src/Functional/CommentLanguageTest.php ->patched
/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php -> new created

Can somebody explain what do I have to do in order to correct the issues in a production site? Do I need to patch only the first file or the other two files are also needed for a functional website? The last 2 files are created just for testing?

UPDATE: I applied the patch only in the first file and it works for submit. Preview of the comment in the translated page doesn't work though. Brings a white page. Delete a comment on a tranlated page redirects to the non-translated page. So 2 out of 3 major issues are not yet solved. By viewing all the above posts I don't understand why we spent so much effort in the testing files and not in the real issues which are the following:
1. Redirect after save (patch looks working)
2. Loading preview (no solution at the moment -> white screen of death)
3. Redirect after delete (no solution at the moment -> still redirects to the non translated page)

We are in post #90 and only the first issue is resolved and this thread is 4 years old!!! I consider the issue major and I really am surprised why this hasn't been solved in the core already! This is basic functionality if you want to have a multilingual site. Am I missing something? How are all the rest working on these issues? Is it because D7 was working fine in all these and D8/D9 changed the architecture and produced new issues?

Created a new topic here to raise general awareness for all these issues the comment module has https://www.drupal.org/project/drupal/issues/3196886

So as it is the usual case with Drupal, an issue takes years to get solved I decided to solve the issues myself in order to proceed with my project. Here are the modifications I did. I'm not a professional Drupal programmer so I'll place here the code without any code standards. A more qualified person should then review my suggestions and provide them in the form of patches. My modifications presented here address the following issues in version 9.1.2 of Drupal:

1. Comment Save redirect in a translated page
2. Comment Delete redirect in a translated page
3. Comment Preview in a translated page

For the first issue (save) in file /core/modules/comment/src/CommentForm.php in function "save"

@line 372 - $entity = $comment->getCommentedEntity();
@line 372 + $entity = $this->entityRepository->getTranslationFromContext($comment->getCommentedEntity());

For the second issue (delete) in file /core/modules/comment/src/Form/DeleteForm.php in function "getCancelUrl"

@line 19 - return $this->entity->get('entity_id')->entity->toUrl();
@line 19 + return $this->entity->get('entity_id')->entity->toUrl('canonical', ['language' => \Drupal::languageManager()->getCurrentLanguage()]);

For the third issue (preview) in file /core/modules/comment/comment.module in function "comment_preview"

@line 496 - $entity = $comment->getCommentedEntity();
@line 496 + $entity = \Drupal::service('entity.repository')->getTranslationFromContext($comment->getCommentedEntity());

Please, let's test the above and produce the corresponding patches, and let's hope it will not take 4.5years more...

Updated the patch from #83 to make it compatible with Drupal core version 9.1.4

Well, fixed the file locations in my patch from previous comment.

Changing to needs review for test bot run.

+++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
@@ -0,0 +1,128 @@
+    $assert_session->addressEquals('mr/node/1#comment-2');

It seems tests are silently failing and working fine on local.

As drupal's test bot runs test cases inside a subdirectory, this might be causing issues in similar assertions.

I've updated the patch and using Url::fromRoute to create the URLs.

Let's see how it goes.

Rerolled #98 for Drupal 9.2.2

Just changed row number and added an 's' twice.

Great it worked for me in version 8.9.7

1. +++ b/core/modules/comment/tests/src/Functional/CommentLanguageTest.php
   @@ -146,9 +146,9 @@ public function testCommentLanguage() {
            $comment = Comment::load(reset($cids));
   -        $args = ['%node_language' => $node_langcode, '%comment_language' => $comment->langcode->value, '%langcode' => $langcode];
   -        $this->assertEquals($langcode, $comment->langcode->value, new FormattableMarkup('The comment posted with content language %langcode and belonging to the node with language %node_language has language %comment_language', $args));
   -        $this->assertEquals($comment_values[$node_langcode][$langcode], $comment->comment_body->value, 'Comment body correctly stored.');
   +        $args = ['%node_language' => $node_langcode, '%comment_language' => $comment->get('langcode')->value, '%langcode' => $langcode];
   +        $this->assertEquals($comment->get('langcode')->value, $langcode, new FormattableMarkup('The comment posted with content language %langcode and belonging to the node with language %node_language has language %comment_language', $args));
   +        $this->assertEquals($comment->get('comment_body')->value, $comment_values[$node_langcode][$langcode], 'Comment body correctly stored.');
          }
        }
    
   

   This doesn't seem to make any relevant change?

   Order argument is incorrectly changed on assertEquals() due to the merge ,and the only other chnage is $node->langcode to $node->get('langcode'). I prefer get() too but functionally it is identical.

   Lets try without these changes, I don't see why it wouldn't pass?

2. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,129 @@
   +
   +    // Add languages.
   +    foreach (['mr', 'fr'] as $lang) {
   +      ConfigurableLanguage::createFromLangcode($lang)->save();
   +    }
   

   $lang is an uncommon variable name, we usually use langcode. I would skip the loop and just call createFromLangcode() twice.

3. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,129 @@
   +  /**
   +   * Tests redirections with posting comments on multilingual content.
   +   *
   +   * @throws \Behat\Mink\Exception\ExpectationException
   +   */
   +  public function testCommentOnMultilingualContent() {
   +
   +    // Create multilingual content.
   

   I wouldn't add the @throws on these methods. If you start with that then you'd need to add a lot more from the assertions to be complete. We don't do that on test methods usually.

4. +++ b/core/modules/comment/tests/src/Functional/CommentOnMultilingualContentTest.php
   @@ -0,0 +1,129 @@
   +    $node
   +      ->addTranslation('mr', ['title' => 'Marathi title'])
   +      ->addTranslation('fr', ['title' => 'French title'])
   +      ->save();
   

   Hm. So what is the expected behavior if you comment with a language that does _not_ have an active translation?

   I would argue that we should actually set the language explicitly even if a translation doesn't exist for it?

   For the test, I would then explicitly _not_ add a fr translation and still test it like that.

patch not applied in drupal-9.3.x-dev
Needs to re-roll

Removing credit for #104, a screenshot of a patch not applying does not move the issue forward.

The testbot in #101 is able to apply the patch so clearly it's working.

The screenshot does show that the patch you're attempting to apply is not the most recent - it's trying to apply #88 and it's been rerolled since then - the most recent is #101. You should only apply the most recent patch when reviewing, it is not cumulative. The use of bold is because I've mentioned this on several other issues and you may have missed it.

Addressed #103:

1. Changes reverted
2. Done
3. Removed all @throws \Behat\Mink\Exception\ExpectationException
4. I hope I understood this correctly

Had some problems with this issue as well. Don't want to duplicate my comment, so I'll just leave a link here - https://www.drupal.org/project/drupal/issues/2751267#comment-14810709

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Did not read all the comments yet.

But see there are few items in the remaining tasks to be addressed.
Also was tagged for a follow up was that done?

Can re-review later once those are updated

Thanks.

The patch fixes the issue of the page returning to the default language page after submitting a comment in the non-default language comment form. It did not resolve other problems related to previewing comments and deleting comments. The patch is for Drupal development branch 10.2.x.

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Thanks for picking up

MR should be pointed at 11.x branch

Appears to have some remaining tasks that need to be addressed or answered.