Use Xss::filter() for the view title to ensure that the preview matches the actual display

I recommend you to use CSS to do those styles changes

Updated IS for clarity around the issue.

Hi @pameeela,
Here I have changed the markup to plain text in the preview section. Now Whatever tags we will give in the text field, it will strip those tags and print as a plain text in Preview section. While viewing the page, it will come along with the h2 tag which is by default added in the twig file. Please let me know if there are any issues.

Applied patch #8. It works fine. The markup of the view title in preview will be removed after this patch

Screenshots:
before patch

After patch

+++ b/core/modules/views_ui/tests/src/Functional/PreviewTest.php
@@ -163,4 +163,14 @@ public function testPreviewError() {
+  /**
+   * Tests view title in the preview
+   */
+  public function testPreviewTitle() {
+    $title = "<h2>View Preview Title</h2>";
+    $this->drupalGet('admin/structure/views/view/test_preview_title/edit');
+    $this->assertEquals("View Preview Title", strip_tags($title));
+
+  }

This is testing the strip_tags method (it works!), the drupalGet doesn't do anything to influence the test. We need to assert that the striped text appears on the page. And in this case, it won't, because the 'test_preview_title' View that is enabled doesn't actually exist, so the drupalGet will see a 404

updating patch as per #12 suggestion.

#13 is missing tests entirely.

What is the purpose of having HTML tags in a view title if they are always stripped?
It doesn't make much sense to me.

Agreed with @abramm

Here's a test-only patch + a patch with the existing fix which will also fail.

There are 2 places that need tags stripped - the preview section and the query info section. The existing fix only removes it from the former.

However, in my testing the following statement is not true:

You can add HTML tags to a view title, and they work as expected in the view preview, but are stripped from the display.

The tags are NOT stripped from the display when the view is actually rendered (not in preview). So is this actually a valid bug?

Screenshots to show the above:

Before patch:

After patch:

HTML of Views output with HTML included in title:

This strips the HTML tags from the query info section as well.

So, it works for <strong> tags, but not for <b> tags (just triple confirmed this and that was the original issue). I assume that this means some tags *are* allowed?

Any way to find out which and then evaluate whether this works as designed, or needs to change?

Updated IS to reflect the latest findings.

So this needs to be updated to use Xss::filter and not strip_tags and then we need to test that the right tags get stripped.

Made the changes suggested in #25. Please review.

+++ b/core/modules/views_ui/src/ViewUI.php
@@ -690,7 +690,7 @@ public function renderPreview($display_id, $args = []) {
+                  '#markup' => strip_tags($executable->getTitle()),

+++ b/core/modules/views_ui/views_ui.module
@@ -134,7 +134,7 @@ function views_ui_preprocess_views_view(&$variables) {
+      '#markup' => filter_var($view->getTitle()),

@ayushmishra206 You've only updated it in one spot and not with the method we were talking about.....

+++ b/core/modules/views_ui/src/ViewUI.php
@@ -690,7 +690,7 @@ public function renderPreview($display_id, $args = []) {
+                  '#markup' => filter_var($executable->getTitle()),

+++ b/core/modules/views_ui/views_ui.module
@@ -134,7 +134,7 @@ function views_ui_preprocess_views_view(&$variables) {
+      '#markup' => filter_var($view->getTitle()),

It's still the wrong method, we were talking about Xss::filter().....

Sorry for the mistakes.

Patch looks good and applied cleanly. moving to RTBC.

Looks good too. RTBC +1

+++ b/core/modules/views_ui/tests/src/Functional/PreviewTest.php
@@ -163,4 +170,23 @@ public function testPreviewError() {
+    \Drupal::configFactory()->getEditable('views.view.test_preview_title')
+      ->set('display.default.display_options.title', '<b>Test preview title</b>')
+      ->save();

This looks good but couldn't we re-use the test_preview View for this instead of adding a whole new View?

@Lendude totally could, but I've done similar before and was told to create a dedicated config for the specific test. I guess it means that if the re-used view was changed in any way it wouldn't affect our test but given we are testing a pretty small part of the view it might be ok.

+++ b/core/modules/views_ui/src/ViewUI.php
@@ -690,7 +691,7 @@ public function renderPreview($display_id, $args = []) {
-                  '#markup' => $executable->getTitle(),
+                  '#markup' => Xss::filter($executable->getTitle()),

+++ b/core/modules/views_ui/views_ui.module
@@ -134,7 +135,7 @@ function views_ui_preprocess_views_view(&$variables) {
-      '#markup' => $view->getTitle(),
+      '#markup' => Xss::filter($view->getTitle()),

Atm these are both admin XSS filtered - all markup is. This could be achieved with

  '#markup' => $view->getTitle(),
  '#allowed_tags' => Xss::getHtmlTagList(),

for less Xss filtering - atm this string is being filtered twice.

I think not adding a whole new view is okay here too.

One question I have is how are we doing the eventual Xss filtering here. And I've found it... it's in \Drupal\views\Plugin\views\display\Page::execute() where we do...

      $render += [
        '#title' => ['#markup' => $this->view->getTitle(), '#allowed_tags' => Xss::getHtmlTagList()],
      ];

One funny thing is that $this->view->getTitle() will Xss admin filter the title already...

- Remove the additional view.
- Perform the relevant changes in test cases, so we can test it with test_preview view.
- Add '#allowed_tags' => Xss::getHtmlTagList(), at both the places.
- Test cases are passing on local with test_preview view.

The patch #41 Applied successfully and the view title not appearing in bold now. Also it is not showing in bold when block placed in any region.

Rerolled #41 for 9.4.

Patch #44 Applied and Tested successfully on Drupal 9.4.x.

Before patch

After Patch

Applied patch #44 successfully on drupal version 9.4.x and working fine. The markup of the view title in preview will be removed after this patch.
Refer to screenshot

Thank you @gaurav-mathur but screenshots were already done in #46 so that was duplicate effort

Reviewing patch #44

On Drupal 10.1 with a standard install

Ran tests locally without the fix as I didn't see a test-only

Failed asserting that actual size 2 matches expected size 0.

So that's good

Double checked #46 and confirmed the issue is fixed.

Reviewing the code nothing seems off. Since this is the preview only I can't imagine this will break anything existing.

Thanks!

Picking up again.
I tested the patch on local and it is passing the test cases added in re-roll #44
Patch is also getting applied cleanly on 10.1.x branch and passing on local on 10.1.x branch.

I've triggered the test-bot again as previous run's failures were related to date module.
Marking it as RTBC again so bot can pick up on next cycle.

Committed and pushed f953b423 to 10.1.x and 463dbb6b43 to 10.0.x and 1ae1111127 to 9.5.x. Thanks!