Views UI generates translated HTML class names for handlers / buckets

Tours are not content, so #2363099: Using translated field definition descriptions in entity schema results in different schema definitions, resulting in update.php changes should not be related. I'm surprised if this this happens because we merge translations into original config, so it should not remove items.

Please export all your configuration at Admin > Configuration > Configuration management. You get a package download. Unzip that and look at the language directory and the yml files there. Can you post the two overrides for those two languages here?

So the Afrikaans has one item in the override:

tips:
  views-ui-preview:
    label: Voorskou

The Dutch one has three:

tips:
  views-ui-format:
    label: Weergaveformaat
  views-ui-fields:
    label: Velden
  views-ui-preview:
    label: Voorbeeldweergave

Neither has 10 page titles translated. In fact the Afrikaans has less translated. So that that would display 10 and the Dutch would display 4 sounds strange still :) I am glad it is reproducible.

Looked into this. Tour steps are generated into the page markup at the end:

<div class="item-list"><ol id="tour" class="hidden"><li ....</div>

When you load the bogus page BEFORE you hit the Tour button, it has 10 steps, ie "4 van 10", "8 van 10" etc. When you hit the tour button, it is reduced to 4 steps visible int the rewritten page as well.

tour.js has a _removeIrrelevantTourItems() method which does the removal of items based on some criteria and the renumbering of steps which is happening in this case.

Indeed, what I found stepping into this is that the tour items with ID attachments were kept. The tour items with class attachments are not. The classes in the markup are translated. Should not be:

I think this may lead to bigger problems, therefore major.

@balagan: those are the class name that are being looked for (as defined in the tour). Go up in the markup and look at the class names in views. The problem is tour looks for class names not in the document, because views translates them.

The bug seems to be in core/modules/views_ui/templates/views-ui-display-tab-bucket.html.twig:

/**
 * ... Available variables:
 * - name: The name of the bucket, e.g. "Fields", "Filter Criteria", etc. ...
 */
#}
{%
  set classes = [
    name ? name|clean_class,
  ]
%}
<div{{ attributes.addClass(classes) }}>
...
</div>

Better title. The translated handler / bucket names come from eg. Views.php (static::t('Sort criteria')), (static::t('Filter criteria')) as well as displays such as DisplayPluginBase ($this->t('Format')).

Updated issue summary.

Not sure if this is the way since the class names are not so descriptive anymore to go but this fixes it.

@geertvd: that seems to be failing on this test:

    $areas = array(
      'header' => 'header',
      'footer' => 'footer',
      'empty' => 'no-results-behavior',
    );

    // Assert that the expected text is found in each area category.
    foreach ($areas as $type => $class) {
      $element = $this->xpath('//div[contains(@class, :class)]/div', array(':class' => $class));
      ....
    }

Apparently changing those class names wasn't a good idea since this breaks some other tests.
In this one I'm just passing the untranslated title along.

ignore interdiff failing, I should use txt extension there.

Good catch!

Nice find. Looks like a test would be good.

Added test to ViewsUITourTest

Looks really good. Some notes:

1. +++ b/core/modules/views_ui/src/Tests/ViewsUITourTest.php
   @@ -50,4 +51,77 @@ public function testViewsUiTourTips() {
   +    $ln = 'nl';
   

   Let's name this variable $langcode, that is our usual variable naming for this.

2. +++ b/core/modules/views_ui/src/Tests/ViewsUITourTest.php
   @@ -50,4 +51,77 @@ public function testViewsUiTourTips() {
   +    $this->importPoFile($this->getPoFile(), array(
   +        'langcode' => $ln,
   +      ));
   

   Indentation is off. Should not end up indented twice.

3. +++ b/core/modules/views_ui/src/Tests/ViewsUITourTest.php
   @@ -50,4 +51,77 @@ public function testViewsUiTourTips() {
   +    $this->assertTourTips();
   

   Wow I did not know this exists. Wow. Huh. Nice.

1. Fixed
2. Fixed
3. Stumbled upon that one by accident, my test looked totally different before that.

Looks great, thanks!

+++ b/core/modules/views/src/Views.php
@@ -419,6 +419,7 @@ public static function pluginList() {
+   *   - otitle: The original untranslated title of the handler type.

I'm not sure that adding yet another thing to this array is worth it. How about using the key instead? Yes that will change the class names but we storing the original title for class names feels ott.

I agree, should have sticked with my thoughts on #24 and just fix the breaking tests.
Hope this one passes.

Now that we are modifying CSS classes, are there CSS selector matching to those classes?

Found one in a js file, this is supposed to hide the "contextual filter"-textfield in preview but that feature seems broken perhaps a new (minor) issue should be opened for that?

I also looked around and did not find other CSS/JS affected. This should be all good now.

+++ b/core/modules/views_ui/src/Tests/ViewsUITourTest.php
@@ -50,4 +51,77 @@ public function testViewsUiTourTips() {
+    // Add Dutch language programmatically.
+    ConfigurableLanguage::createFromLangcode($langcode)->save();
+    // Import a .po file.
+    $this->importPoFile($this->getPoFile(), array(
+      'langcode' => $langcode,
+    ));
+

Is there a reason we don't just save the translated strings via an API function?

Yeah that is possible to do, although that test would need to make sure to clear all caches proper to reproduce the bug then which may not be very nice either :) #1785086: Introduce a generic API for interface translation strings provided an API.

Fixed this based based on #1785086: Introduce a generic API for interface translation strings. I didn't have to clear the cache to make this work locally though. Don't know if the testbot is going to do this differently.

Looks good. Indeed simpler than importing a .po file.

Great, thank you!

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed fca50d2 and pushed to 8.0.x. Thanks!

Yay and congrats geertvd, balagan!