Files: 
CommentFileSizeAuthor
#25 2086485-25.patch20.24 KBpwolanin
PASSED: [[SimpleTest]]: [MySQL] 60,117 pass(es). View
#19 drupal8.language-module.2086485-19.patch25.32 KBjibran
FAILED: [[SimpleTest]]: [MySQL] 59,123 pass(es), 57 fail(s), and 0 exception(s). View
#19 interdiff.txt5.44 KBjibran
#16 drupal8.language-module.2086485-16.patch25.04 KBdisasm
PASSED: [[SimpleTest]]: [MySQL] 58,778 pass(es). View
#16 interdiff.txt907 bytesdisasm
#14 drupal8.language-module.2086485-14.patch25.04 KBjibran
FAILED: [[SimpleTest]]: [MySQL] 58,805 pass(es), 6 fail(s), and 2 exception(s). View
#14 interdiff.txt905 bytesjibran
#12 drupal8.language-module.2086485-12.patch25.04 KBjibran
FAILED: [[SimpleTest]]: [MySQL] 59,175 pass(es), 0 fail(s), and 5 exception(s). View
#12 interdiff.txt9.39 KBjibran
#6 drupal8.language-module.2086485-6.patch23.62 KBbrianV
PASSED: [[SimpleTest]]: [MySQL] 59,129 pass(es). View
#4 drupal8.language-module.2086485-4.patch24.71 KBbrianV
FAILED: [[SimpleTest]]: [MySQL] 58,912 pass(es), 14 fail(s), and 4 exception(s). View
#3 drupal8.language-module.2086485-3.patch24.74 KBbrianV
FAILED: [[SimpleTest]]: [MySQL] 58,908 pass(es), 14 fail(s), and 4 exception(s). View
#2 drupal8.language-module.2086485-2.patch24.74 KBbrianV
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

Comments

brianV’s picture

Assigned:Unassigned» brianV

I'll take a crack at this in the morning!

brianV’s picture

Status:Active» Needs review
FileSize
24.74 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion. View

First pass

brianV’s picture

FileSize
24.74 KB
FAILED: [[SimpleTest]]: [MySQL] 58,908 pass(es), 14 fail(s), and 4 exception(s). View

With whitespace fixes dreditor hinted at.

brianV’s picture

FileSize
24.71 KB
FAILED: [[SimpleTest]]: [MySQL] 58,912 pass(es), 14 fail(s), and 4 exception(s). View

Sorry, one final niggling whitespace fix.

Status:Needs review» Needs work

The last submitted patch, drupal8.language-module.2086485-4.patch, failed testing.

brianV’s picture

Status:Needs work» Needs review
FileSize
23.62 KB
PASSED: [[SimpleTest]]: [MySQL] 59,129 pass(es). View

This patch backs off the Local Tasks changeover as that's still in flux and was causing test failures.

The patch has two failures left that I can't figure out. Apparently, somehow, my changes to the language negotiation form controller cause the site to redirect incorrectly after the a different default language is selected on the regional settings page. It's not clear how changes to this route and controller are affecting that one.

Open to any ideas about these failures.

Status:Needs review» Needs work

The last submitted patch, drupal8.language-module.2086485-6.patch, failed testing.

jibran’s picture

Status:Needs work» Needs review
brianV’s picture

jibran - thanks for updating the status.

It was inexplicably failing tests; now it works! Must have been a bad test or something that got fixed...

dawehner’s picture

  1. +++ b/core/modules/language/lib/Drupal/language/Form/NegotiationConfigureForm.php
    @@ -0,0 +1,241 @@
    +   * Implements \Drupal\Core\Form\FormInterface::getFormID().
    ...
    +   * Implements \Drupal\Core\Form\FormInterface::buildForm().
    ...
    +   * Implements \Drupal\Core\Form\FormInterface::submitForm().
    

    @inheritdoc

  2. +++ b/core/modules/language/lib/Drupal/language/Form/NegotiationConfigureForm.php
    @@ -0,0 +1,241 @@
    +    $configurable = \Drupal::config('system.language.types')->get('configurable');
    ...
    +    $stored_values = \Drupal::config('system.language.types')->get('configurable');
    

    Isn't there $this->config() available?

  3. +++ b/core/modules/language/lib/Drupal/language/Form/NegotiationConfigureForm.php
    @@ -0,0 +1,241 @@
    +    // Update non-configurable language types and the related language negotiation
    +    // configuration.
    ...
    +      // If there is an active language switcher for a language type that has been
    +      // made not configurable, deactivate it first.
    

    >80 chars.

  4. +++ b/core/modules/language/lib/Drupal/language/Form/NegotiationConfigureForm.php
    @@ -0,0 +1,241 @@
    +      \Drupal::service('plugin.manager.block')->clearCachedDefinitions();
    

    Is there a reason to not inject it?

  5. +++ b/core/modules/language/lib/Drupal/language/Form/NegotiationConfigureForm.php
    @@ -0,0 +1,241 @@
    +    drupal_set_message(t('Language negotiation configuration saved.'));
    

    All of the t() should be $this->t()

jibran’s picture

Status:Needs review» Needs work

NW as per #10.

jibran’s picture

Status:Needs work» Needs review
FileSize
9.39 KB
25.04 KB
FAILED: [[SimpleTest]]: [MySQL] 59,175 pass(es), 0 fail(s), and 5 exception(s). View

Some fixes.

Status:Needs review» Needs work

The last submitted patch, drupal8.language-module.2086485-12.patch, failed testing.

jibran’s picture

Status:Needs work» Needs review
FileSize
905 bytes
25.04 KB
FAILED: [[SimpleTest]]: [MySQL] 58,805 pass(es), 6 fail(s), and 2 exception(s). View

:/

Status:Needs review» Needs work

The last submitted patch, drupal8.language-module.2086485-14.patch, failed testing.

disasm’s picture

FileSize
907 bytes
25.04 KB
PASSED: [[SimpleTest]]: [MySQL] 58,778 pass(es). View

slight typo, this should fix it.

disasm’s picture

Status:Needs work» Needs review
disasm’s picture

+++ w/core/modules/language/lib/Drupal/language/Form/NegotiationConfigureForm.php
@@ -0,0 +1,296 @@
+  /**
+   * Stores the configuration configuration object for system.language.types.
+   *
+   * @var \Drupal\Core\Config\Config
+   */
+  protected $config;
...
+    $this->config = $config_factory->get('system.language.types');

I'd prefer to store the entire configFactory so if another module extends this, and needs a different config, they'd be able to get it still, without reinjecting the configFactory. If you aren't going to store the factory, at least specify what config this is instead of just $config.

jibran’s picture

FileSize
5.44 KB
25.32 KB
FAILED: [[SimpleTest]]: [MySQL] 59,123 pass(es), 57 fail(s), and 0 exception(s). View

Some fixes and tested manually works fine.

disasm’s picture

I can live with that. If it passes, RTBC.

disasm’s picture

disasm’s picture

Issue tags:+D8MI

just can't win...

Status:Needs review» Needs work

The last submitted patch, drupal8.language-module.2086485-19.patch, failed testing.

Hydra’s picture

Issue tags:+Needs reroll

Seems like the patch no longer applies

pwolanin’s picture

Issue summary:View changes
Status:Needs work» Needs review
Issue tags:-Needs reroll
FileSize
20.24 KB
PASSED: [[SimpleTest]]: [MySQL] 60,117 pass(es). View

Here's a re-roll and minor tweaks around the handling of the block manager.

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

I manually tested the page and it worked as expected. The code looks fine as well.

webchick’s picture

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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