Problem/Motivation

1. Install Drupal in German.
2. Create a view. Look at pager settings, they are in English. Save it.
3. All strings are in English but the file is langcode: de.

We don't t() the default options. For example:

abstract class ExposedFormPluginBase extends PluginBase {

  protected function defineOptions() {
    $options = parent::defineOptions();
    $options['submit_button'] = array('default' => 'Apply');
    $options['reset_button'] = array('default' => FALSE);
    $options['reset_button_label'] = array('default' => 'Reset');
    $options['exposed_sorts_label'] = array('default' => 'Sort by');
    $options['expose_sort_order'] = array('default' => TRUE);
    $options['sort_asc_label'] = array('default' => 'Asc');
    $options['sort_desc_label'] = array('default' => 'Desc');
    return $options;
  }
}

abstract class SqlBase extends PagerPluginBase {

  protected function defineOptions() {
    $options = parent::defineOptions();
    $options['items_per_page'] = array('default' => 10);
    $options['offset'] = array('default' => 0);
    $options['id'] = array('default' => 0);
    $options['total_pages'] = array('default' => '');
    $options['expose'] = array(
      'contains' => array(
        'items_per_page' => array('default' => FALSE),
        'items_per_page_label' => array('default' => 'Items per page'),
        'items_per_page_options' => array('default' => '5, 10, 25, 50'),
        'items_per_page_options_all' => array('default' => FALSE),
        'items_per_page_options_all_label' => array('default' => '- All -'),

        'offset' => array('default' => FALSE),
        'offset_label' => array('default' => 'Offset'),
      ),
    );
    $options['tags'] = array(
      'contains' => array(
        'previous' => array('default' => '‹ previous'),
        'next' => array('default' => 'next ›'),
      ),
    );
    return $options;
  }
}

The options are documented to have a 'translatable' key but its not clear how that should be used or what it means. It does not seem to be used either.

  /**
   * Fills up the options of the plugin with defaults.
   *
   * @param array $storage
   *   An array which stores the actual option values of the plugin.
   * @param array $options
   *   An array which describes the options of a plugin. Each element is an
   *   associative array containing:
   *   - default: The default value of one option
   *   - (optional) contains: An array which describes the available options
   *     under the key. If contains is set, the default will be ignored and
   *     assumed to be an empty array.
   *   - (optional) 'translatable': TRUE if it should be translated, else FALSE.
   *   - (optional) 'bool': TRUE if the value is boolean, else FALSE.
   */
  protected function setOptionDefaults(array &$storage, array $options) {

Proposed resolution

Translate default options in their definitions.

Remaining tasks

Discuss. Review.

User interface changes

None.

API changes

The defaultOptions() expected return value slightly changes.

Files: 
CommentFileSizeAuthor
#10 interdiff.txt2.46 KBGábor Hojtsy
#10 2446431-8.patch9.55 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,685 pass(es). View
#6 interdiff.txt2.3 KBGábor Hojtsy
#6 2446431-6.patch10.34 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,691 pass(es). View
#3 interdiff.txt859 bytesGábor Hojtsy
#3 2446431-3.patch8.04 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,693 pass(es), 2 fail(s), and 0 exception(s). View
#2 interdiff.txt4.26 KBGábor Hojtsy
#2 2446431-2.patch7.72 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,682 pass(es), 2 fail(s), and 0 exception(s). View
#1 2446431-1.patch3.45 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,680 pass(es). View

Comments

Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
3.45 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,680 pass(es). View

Changing some random things as a start to see if things blow up.

Gábor Hojtsy’s picture

Issue summary: View changes
FileSize
7.72 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,682 pass(es), 2 fail(s), and 0 exception(s). View
4.26 KB

Looked through all the ones I found. I found one instance of the translatable option. @dawehner confirmed that its a D7 leftover and does not do anything in D8.

Gábor Hojtsy’s picture

FileSize
8.04 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,693 pass(es), 2 fail(s), and 0 exception(s). View
859 bytes

Update API docs.

The last submitted patch, 2: 2446431-2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: 2446431-3.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
10.34 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,691 pass(es). View
2.3 KB

The string translation service is now needed to make the user/node bulk form tests work of course.

dawehner’s picture

The patch looks great in general.

One question is open for me, do we set the language of a view correctly?

+++ b/core/modules/node/tests/src/Unit/Plugin/views/field/NodeBulkFormTest.php
@@ -51,6 +51,9 @@ public function testConstructor() {
+    $translation_manager = $this->getMockBuilder('\Drupal\Core\StringTranslation\TranslationManager')
+      ->disableOriginalConstructor()
+      ->getMock();

@@ -60,6 +63,7 @@ public function testConstructor() {
+    $container->set('string_translation', $translation_manager);

+++ b/core/modules/user/tests/src/Unit/Plugin/views/field/UserBulkFormTest.php
@@ -51,6 +51,9 @@ public function testConstructor() {
+    $translation_manager = $this->getMockBuilder('\Drupal\Core\StringTranslation\TranslationManager')
+      ->disableOriginalConstructor()
+      ->getMock();
     $views_data = $this->getMockBuilder('Drupal\views\ViewsData')

@@ -60,6 +63,7 @@ public function testConstructor() {
+    $container->set('string_translation', $translation_manager);

You can use getStringTranslationStub directly() afaik.

geertvd’s picture

geertvd’s picture

Should I merge the test coverage we have for this issue in #2424445: Views default options such as pager options are not translated?

Gábor Hojtsy’s picture

FileSize
9.55 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,685 pass(es). View
2.46 KB

@dawehner: great tip on getStringTranslationStub(), using that now.

On the other point its not really a question of setting the language on the view correctly, all config entities get created in the site's default language unless explicitly provided otherwise (from ConfigEntityStorage):

  protected function doCreate(array $values) {
    // Set default language to site default if not provided.
    $values += array($this->langcodeKey => $this->languageManager->getDefaultLanguage()->getId());
    $entity = new $this->entityClass($values, $this->entityTypeId);
    return $entity;
  }

The assumption being that on a German site you would create German content types, German fields, German contact categories and German views and then if there are other languages you would translate from there. It is true that if you have a multilingual site and start creating a view on a French page, you would probably expect the view created in French not German, so that may need to be refined a bit, but that does not change whether we need to translate default values which are translatable AFAIS.

Gábor Hojtsy’s picture

@geertvd: for commit credit fairness it sounds better to fold this issue to that one and not the other way around? If @alexpott accepts the expansion of scope there. I don't think we need to test all options, its nice to have a test for the pager.

geertvd’s picture

Wouldn't it make more sense not to have those defaults in the yml files though? In most core views the defaults in the yml files match the once that are used by interface translation.
So then you don't necessarily need to have config_translation enabled to see those translations since they would be picked up by interface translation.

Gábor Hojtsy’s picture

@geertvd: for shipped views, they are translated either way; if in t() only then runtime when displayed if in the config file then via the locale-config integration. For newly created config, views needs to be able to show you the UI to change the defaults, so it needs to prefill the values. Then its an interesting idea that it would need to verify if you did not change it from what it showed you and then should not save it into the config. I think that may be a bit too much magic, but either way IMHO out of scope for this issue.

geertvd’s picture

Thanks for the explanation.

Then its an interesting idea that it would need to verify if you did not change it from what it showed you and then should not save it into the config.

If you put it like that I agree that this is out of scope :).

Gábor Hojtsy’s picture

Status: Needs review » Closed (duplicate)
Issue tags: -sprint
Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

@dawehner: re whether the default language assumption for config entities is good, I opened #2447139: Config entities should be created in the negotiated language unless otherwise specified and proposed a patch.