Double escaping in views attachment titles

+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_attachment_ui.yml
@@ -34,15 +30,47 @@ display:
+          tokenize: false
+          content:
+            value: 'Attachment title: [view:title]'
+            format: plain_text
+          plugin_id: text

Do you mind having a more explicit test for that?

Tagging for Medium

The core committers and Views maintainers (alexpott, xjm, effulgentsia, tim.plunkett, and dawehner) agreed that this is not a major issue (despite @alexpott originally filing it as such).

Needs work due to #5

Thank you alex!

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

+++ b/core/modules/views/src/Plugin/views/style/StylePluginBase.php
@@ -248,7 +252,7 @@ public function tokenizeValue($value, $row_index) {
       $value = Xss::filterAdmin($value);
     }
-    return $value;
+    return ViewsRenderPipelineMarkup::create($value);
   }
 

This change breaks everything, I have errors like this in some modules, and get's fixed reverting this line.

InvalidArgumentException: $string ("Job overview") must be a string. in Drupal\Core\StringTranslation\TranslatableMarkup->__construct() (line 145 of core/lib/Drupal/Core/StringTranslation/TranslatableMarkup.php).
Drupal\Core\StringTranslation\TranslationManager->translate(Object, Array, Array) (Line: 74)
Drupal\Core\Controller\TitleResolver->t(Object, Array, Array) (Line: 71)
Drupal\Core\Controller\TitleResolver->getTitle(Object, Object) (Line: 158)

OK reverted from 8.0.x for now.

Leaving this in 8.1.x for a bit longer - can you post a longer backtrace or example code that's triggering this?

The full backtrace:

The website encountered an unexpected error. Please try again later.

InvalidArgumentException: $string ("Job overview") must be a string. in Drupal\Core\StringTranslation\TranslatableMarkup->__construct() (line 145 of core/lib/Drupal/Core/StringTranslation/TranslatableMarkup.php).
Drupal\Core\StringTranslation\TranslationManager->translate(Object, Array, Array) (Line: 74)
Drupal\Core\Controller\TitleResolver->t(Object, Array, Array) (Line: 71)
Drupal\Core\Controller\TitleResolver->getTitle(Object, Object) (Line: 158)
Drupal\system\PathBasedBreadcrumbBuilder->build(Object) (Line: 88)
Drupal\Core\Breadcrumb\BreadcrumbManager->build(Object) (Line: 77)
Drupal\system\Plugin\Block\SystemBreadcrumbBlock->build() (Line: 207)
Drupal\block\BlockViewBuilder::preRender(Array)
call_user_func('Drupal\block\BlockViewBuilder::preRender', Array) (Line: 384)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 451)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 200)
Drupal\Core\Render\Renderer->render(Array) (Line: 468)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 59)
__TwigTemplate_999eccb5dced5a7ce436f58b233e3aa534ddbf60db93110b5b36d33caf525f35->doDisplay(Array, Array) (Line: 381)
Twig_Template->displayWithErrorHandling(Array, Array) (Line: 355)
Twig_Template->display(Array) (Line: 366)
Twig_Template->render(Array) (Line: 64)
twig_render_template('core/themes/seven/templates/page.html.twig', Array) (Line: 390)
Drupal\Core\Theme\ThemeManager->render('page', Array) (Line: 438)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 200)
Drupal\Core\Render\Renderer->render(Array) (Line: 468)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 88)
__TwigTemplate_3d56bb3d650d04cb22757e6c1835d86e0100783da675473bc3e3b4231571b121->doDisplay(Array, Array) (Line: 381)
Twig_Template->displayWithErrorHandling(Array, Array) (Line: 355)
Twig_Template->display(Array) (Line: 366)
Twig_Template->render(Array) (Line: 64)
twig_render_template('core/themes/classy/templates/layout/html.html.twig', Array) (Line: 390)
Drupal\Core\Theme\ThemeManager->render('html', Array) (Line: 438)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 200)
Drupal\Core\Render\Renderer->render(Array) (Line: 152)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 577)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 153)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 95)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object) (Line: 116)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 144)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 62)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 62)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 103)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 82)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 55)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 637)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I was opening a form that changes the title and adds breadcrumbs.
It was in TMGMT but also it happened to a coworker that was working in this Core issue #2640994: Label token replacement for views entity reference arguments not working

Debugged and if you do a string cast on '_title' => (string) $this->view->getTitle(), in \Drupal\views\Plugin\views\display\PathPluginBase::getRoute() (line 137) it fixes the bug.

OK that looks like we're missing some core test coverage then. Reverted from 8.1.x as well.

Yes, we do, no idea why none of our tests are failing.

Also, it's wrong that this string is being translated in the first place. It is user provided data and shouldn't be run through t(). That's an existing problem but it might be the proper way to solve this. TitleResolver::getTitle() needs a way to not run _title through t(), based on some flag I guess.

Or views needs to use a _title_callback instead.

@edurenye @berdir
Do you mind providing the view which caused the issue?

_title_callback

Well, in general we cannot do that for everything as the computational overhead would be too crazy. We though add some static title in there:

  protected function getRoute($view_id, $display_id) {
    $defaults = array(
      '_controller' => 'Drupal\views\Routing\ViewPageController::handle',
      '_title' => $this->view->getTitle(),
      'view_id' => $view_id,
      'display_id' => $display_id,
      '_view_display_show_admin_links' => $this->getOption('show_admin_links'),
    );

To reproduce, just apply the patch, clear cache and go to d8/admin/content/comment. Boom. Any page that has a view in the path parents causes this.

wow, I would have expected we have test coverage for that page.

I've hit this as well on admin/content - but not sure how I got it like that. I have a dump where it fails, but after simply clearing cache or saving the view without changing anything it's fine. Will try this patch on to see if it's ok without clearing the cache.

Do we need explicit (web) test coverage for that bug?

Yeah I guess @berdir is totally right here.

Running into this issue with a specific Media Library view in 8.6.4 and hoping this patch will fix it. Since the patch didn't apply cleanly for 8.6.4, I re-rolled it.

this is a patch I used to make it work but it works :-) simply convert if it's not string to string.

Sorry here's the patch using composer or direct core

The core was there my bad in the path here's the new one

path fix

works on composer the last patch but here's the one that works on jenkins

Thanks to danflanagan8 for teaching what a Views 'attachment' is.

I tested this on 9.3.x, standard install, and the problem persists. One thing that was unexpected is the title is not displayed on the view, only on the preview of the attachment.

Not sure if this should be in the view_ui component or not.

We did a pretty generic fix here, #2872322: Views preview title is double escaped, so a little surprised that attachments are still broken

Re-rolling patch in #47 to use PHP __toString() method to get a cleaner string representation versus an object serialization.