Invalid TranslatorInterface type hinting in LanguageRequestSubscriber:

  /**
   * The translation service.
   *
   * @var \Drupal\Core\Translation\Translator\TranslatorInterface
   */
  protected $translation;
  /**
   * Constructs a LanguageRequestSubscriber object.
   *
   * @param \Drupal\language\ConfigurableLanguageManagerInterface $language_manager
   *   The language manager service.
   * @param \Drupal\language\LanguageNegotiatorInterface
   *   The language negotiator.
   * @param \Drupal\Core\Translation\Translator\TranslatorInterface $translation
   *   The translation service.
   * @param \Drupal\Core\Session\AccountInterface $current_user
   *   The current active user.
   */
  public function __construct(ConfigurableLanguageManagerInterface $language_manager, LanguageNegotiatorInterface $negotiator, TranslatorInterface $translation, AccountInterface $current_user) {
    $this->languageManager = $language_manager;
    $this->negotiator = $negotiator;
    $this->translation = $translation;
    $this->currentUser = $current_user;
  }

They should be

Drupal\Core\StringTranslation\Translator\TranslatorInterface;
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cs_shadow’s picture

Status: Active » Needs review
FileSize
1.21 KB

Attaching patch with suggested changes in TranslatorInterface type hinting.

penyaskito’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Quick fix
alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Can we also fix this in TranslationManager too - where there are several instances of incorrect type hinting in the same way.

cs_shadow’s picture

Status: Needs work » Needs review
FileSize
10.74 KB

Attached patch fixes the same issue in TranslationManager as well as LanguageRequestSubscriber. Couldn't find any more such instances.

penyaskito’s picture

Status: Needs review » Needs work
+++ b/core/modules/color/color.js
@@ -142,14 +142,14 @@
+            for (j = i + 1; true; ++j) {
...
+            for (j = i - 1; true; --j) {

+++ b/core/modules/color/color.module
@@ -509,7 +510,6 @@ function _color_rewrite_stylesheet($theme, &$info, &$paths, $palette, $style) {
-

@@ -620,7 +620,7 @@ function _color_render_images($theme, &$info, &$paths, $palette) {
+    // Set standard file permissions for webserver-generated files.

There are a lot of unrelated changes that shouldn't be in this patch.

Even if they look nice, they should have their own issues.

cs_shadow’s picture

Status: Needs work » Needs review
FileSize
2.33 KB

Sorry for the last patch. Forgot to checkout after working on another issue. Those extra changes were of another issue. Attached is the correct patch.

Mixologic’s picture

Patch looked good. I did notice that the @return statement in TranslationManager was wrong, so I fixed that as well.

ericski’s picture

Status: Needs review » Needs work

Patch no longer applies on latest git pull

error: core/modules/language/lib/Drupal/language/EventSubscriber/LanguageRequestSubscriber.php: No such file or directory

penyaskito’s picture

penyaskito’s picture

Actually, probably is easier to just recreate the patch from scratch. Feel free to do it, ericski :)

ericski’s picture

Had to catch a plane yesterday, but that sounds great. I'll give it ago tonight unless someone at the extended sprints wants to take it over.

JacobSanford’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.51 KB

I had a few minutes and took this on, recreated from scratch. No interdiff attached.

Mixologic’s picture

Status: Needs review » Reviewed & tested by the community

LGTM.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7b8d437 and pushed to 8.x. Thanks!

  • Commit 7b8d437 on 8.x by alexpott:
    Issue #2239425 by cs_shadow, Mixologic, JacobSanford | penyaskito:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.