Report by @sun on #2209595: Remove drupal_set_title(), drupal_get_title() and associated tests.

Drupal\Core\Utility\Title just contains constants; that should not exist — let's create a follow-up issue to eliminate that utility class and move those constants to a proper home. It would actually be best if those constants would not exist in the first place; now that we have class methods, we can simply have separate getRawTitle(), getPlainTitle(), getXssSafeTitle() methods.

CommentFileSizeAuthor
#5 2214525-5.patch2 KBPalashvijay4O
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Given that the HtmlFragment is a value object this seems doable.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

The only use of this is in Views - #2067931: Use the new title method on views pages.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Priority: Normal » Major
Status: Active » Postponed
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Postponed » Active
Issue tags: +Novice

That other issue has been closed.

There is still a usage of this left in \Drupal\views\Plugin\views\area\Title, but that second argument that it is passing along does not even exist anymore, so it is broken anyway.

Making active and tagging Novice, remove the file and usage of it in that other class (there is also a left-over use somewhere else).

We then need to verify that title area works as expected and fix it if necessary.

Palashvijay4O’s picture

Palashvijay4O CreditAttribution: Palashvijay4O commented
Status: Active » Needs review
FileSize
2214525-5.patch2 KB

A patch .

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Assigned: Unassigned » dawehner

Patch looks good to me. As mentioned above, the title area plugin might be broken, but this doesn't make it any worse than it already is. But lets confirm this with @dawehner.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/views/src/Plugin/views/area/Title.php
@@ -54,7 +53,7 @@ public function preRender(array $results) {
-      $this->view->setTitle($this->sanitizeValue($value, 'xss_admin'), UtilityTitle::PASS_THROUGH);
+      $this->view->setTitle($this->sanitizeValue($value, 'xss_admin'));

Ha, ViewExecutable::setTitle does not deal anymore with the variable itself.

Later in Page::execute() we use Xss::filterAdmin() so the title plugin does not seems to be broken.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Assigned: dawehner » Unassigned

.

chx’s picture

chx CreditAttribution: chx commented

> Ha, ViewExecutable::setTitle does not deal anymore with the variable itself.

Meaning, this is correct since ViewExecutable::setTitle is public function setTitle($title) { doesn't need the second argument any more.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: Remove Drupal\Core\Utility\Title » Remove unused Drupal\Core\Utility\Title
Category: Task » Bug report
Priority: Major » Normal
Status: Reviewed & tested by the community » Fixed

Can't quite parse how this is major, but it's removing dead code so it's a legit bug fix.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed ee5af45 on 8.0.x 
    Issue #2214525 by Palashvijay4O: Remove unused Drupal\Core\Utility\Title

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.