Updated: Comment 0

Problem/Motivation

Since #2032535: Resolve 'title' using the route and render array we can return dynamic titles using the render array.

Proposed resolution

Use both the dynamic title in the render array but also register the static title for convenience.

Remaining tasks

None

User interface changes

None

API changes

None

#2032535: Resolve 'title' using the route and render array

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
Issue tags: +WSCCI
FileSize
6.08 KB

Adding wscci tag and the patch.

Status: Needs review » Needs work

The last submitted patch, vdc-2067931-1.patch, failed testing.

damiankloip’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php
@@ -119,6 +119,11 @@ public function collectRoutes(RouteCollection $collection) {
+    if ($title = $this->getOption('title')) {
+      $route->setDefault('_title', $title);
+    }

Should we always just set this, regardless of whether it's empty of not? What will the default be otherwise?

damiankloip’s picture

Status: Needs work » Needs review
FileSize
1.27 KB
7.36 KB

Rerolled, and fixed the ViewPageControllerTest. I think if we are returning a render array only now, we can just check all the keys we expect are present?

The only other failure now should be the HTTPStatusCode area handler. As it currently relies on setting the statusCode on the response, which we were returning, but not anymore. So we need to deal with this differently.

dawehner’s picture

We could just wait on the HtmlPage patch, which will bring for example http status codes as well. An alternative approach would be to convert the existing usages of changing the response object (content-type for rss/status code for http status code area handler) to use a kernel event subscriber.

Status: Needs review » Needs work

The last submitted patch, 2067931-4.patch, failed testing.

damiankloip’s picture

Could do that, then we would need a view subscriber and a new method that the subscriber would check?

dawehner’s picture

Well, I guess the subscriber would just get the response object and pull it from there?

catch’s picture

Priority: Normal » Major
Issue summary: View changes

Bumping to major, this looks like it needs updating for Twig autoescape/safe markup.

Found this issue via #2214525: Remove unused Drupal\Core\Utility\Title - this is the only remaining use of that.

Wim Leers’s picture

dawehner’s picture

Status: Needs work » Fixed

Most of this is fixed in views already anyway. The related issue is not important for views

xjm’s picture

Status: Fixed » Closed (duplicate)

Marking duplicate then for whatever has been fixed in Views since nothing went in through this issue.

Berdir’s picture

There is still a broken usage of a Utility\Title constant left in Drupal\views\Plugin\views\area\Title but I think we can remove that in #2214525: Remove unused Drupal\Core\Utility\Title, please confirm.