Destination parameter in URL breaks changing a source language

Thanks, @matsbla!

1. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -116,8 +122,10 @@ class ContentTranslationHandler implements ContentTranslationHandlerInterface, E
   +   * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
   +   *   The request stack.
   ...
   -  public function __construct(EntityTypeInterface $entity_type, LanguageManagerInterface $language_manager, ContentTranslationManagerInterface $manager, EntityTypeManagerInterface $entity_type_manager, AccountInterface $current_user, MessengerInterface $messenger, DateFormatterInterface $date_formatter, EntityLastInstalledSchemaRepositoryInterface $entity_last_installed_schema_repository) {
   +  public function __construct(EntityTypeInterface $entity_type, LanguageManagerInterface $language_manager, ContentTranslationManagerInterface $manager, EntityTypeManagerInterface $entity_type_manager, AccountInterface $current_user, MessengerInterface $messenger, DateFormatterInterface $date_formatter, EntityLastInstalledSchemaRepositoryInterface $entity_last_installed_schema_repository, RequestStack $request_stack) {
   

   Needs a CR? Make it optional for BC.

   public function __construct(EntityTypeInterface $entity_type, LanguageManagerInterface $language_manager, ContentTranslationManagerInterface $manager, EntityTypeManagerInterface $entity_type_manager, AccountInterface $current_user, MessengerInterface $messenger, DateFormatterInterface $date_formatter, EntityLastInstalledSchemaRepositoryInterface $entity_last_installed_schema_repository, RequestStack $request_stack = NULL) {
      if(request_stack === NULL) {
        $request_stack = \Drupal::service('request_stack');
      }
   
   

2. +++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationLanguageChangeTest.php
   @@ -117,6 +118,65 @@ public function testLanguageChange() {
   +   * Test changing the source language with destination parameter.
   

   Start with a verb "Test" -> "Tests"

3. +++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationLanguageChangeTest.php
   @@ -117,6 +118,65 @@ public function testLanguageChange() {
   +    $this->drupalPostForm(NULL, [], t('Save (this translation)'));
   ...
   +    $this->drupalPostForm(NULL, $edit, t('Change'), $add_translation_route['options']);
   ...
   +    $this->drupalPostForm(NULL, $edit, t('Save (this translation)'), $add_translation_route['options']);
   

   t() calls should be avoided in tests where possible.

Applying IS template

Not sure if CR is needed as the new construct parameter is optional, doesn't look like CR was created other places where new optional parameters where introduced. Put back to NR to get feedback from a core committer.

@jungle
Thanks for the review!

1. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -127,6 +135,10 @@ public function __construct(EntityTypeInterface $entity_type, LanguageManagerInt
        $this->dateFormatter = $date_formatter;
   +    if ($request_stack === NULL) {
   +     $request_stack = \Drupal::service('request_stack');
   +   }
   +    $this->requestStack = $request_stack;
   

   Wrong indentation. the } line.

2. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -99,6 +100,11 @@ class ContentTranslationHandler implements ContentTranslationHandlerInterface, E
   +   * @var \Symfony\Component\HttpFoundation\RequestStack
   +   */
   

   Missing the summary/comment.

     /**
      * The request stack.
      *
      * @var \Symfony\Component\HttpFoundation\RequestStack
      */
   

3. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -736,15 +749,23 @@ public function entityFormSubmit($form, FormStateInterface $form_state) {
   +    $entity      = $form_object->getEntity();
   +    $source      = $form_state->getValue(['source_langcode', 'source']);
   +    $query       = $this->requestStack->getCurrentRequest()->query;
   +    $options     = [];
   

   Prefer removing extra whitespaces before =, but your way is allowed by coding standard.

Otherwise, besides the CR, it's good to me. Thanks!

Note: attaching a test-only patch would be better.

Thank you!

Looking good, couple of observations/suggestions

1. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -127,6 +137,10 @@ public function __construct(EntityTypeInterface $entity_type, LanguageManagerInt
   +      $request_stack = \Drupal::service('request_stack');
   

   This will need a trigger_error for BC sake to notify of the deprecation for D10

2. +++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationLanguageChangeTest.php
   @@ -117,6 +118,65 @@ public function testLanguageChange() {
   +    $this->assertTrue(strpos($this->getUrl(), $add_translation_href) !== FALSE, 'Correct destination query arguments are found');
   

   We could use assertStringContainsString here instead and then the $message isn't needed

1. Added draft CR: #3189305: \Drupal\content_translation\ContentTranslationHandler constructor adds $request_stack argument.
2. Reroll against 9.2.x including fixing deprecations
3. Addressed both points in #32 in attached patch.

Patch applied cleanly. Moving to RTBC.

Rerolled #33.

CI Failure.