Remove drupal_set_title in help module controllers [#2102453]

Subtask of #1830588: [META] remove drupal_set_title() and drupal_get_title()

Problem/Motivation

Using procedural drupal_set_title() inside controller class is not encouraged.

Proposed resolution

Replace drupal_set_title() with #title in page return array.

Remaining tasks

Issue patch

User interface changes

Refer parent issue at #1830588: [META] remove drupal_set_title() and drupal_get_title()

API changes

Refer parent issue at #1830588: [META] remove drupal_set_title() and drupal_get_title()

Comment	File	Size	Author
#10	drupal8.help-module.2102453-10.patch	1.01 KB	ACF
#10
#8	drupal8.help-module.2102453-8.patch	739 bytes	ACF
#8
#4	drupal-remove_drupal_set_title_in_help_module-2102453-4.patch	851 bytes	InternetDevels
#4
#1	drupal8.help-module.2102453-1.patch	3.46 KB	JulienD
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

JulienD CreditAttribution: JulienD commented 4 October 2013 at 15:16

Status:

Active

» Needs review

File	Size
drupal8.help-module.2102453-1.patch	3.46 KB

Here is a try to remove the drupal_set_title in the help module.

According to https://drupal.org/node/2102369#comment-7930793 I have left the title callback in the hook_menu.

Comment #2

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 4 October 2013 at 16:17

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.php
@@ -118,7 +118,7 @@ public function helpPage($name) {
       if (!empty($admin_tasks)) {
         $links = array();

@@ -142,4 +142,18 @@ public function helpPage($name) {
+    return $this->t($info['name']);

Not sure we really need to t() the dynamic name?

Comment #3

4 October 2013 at 16:57

Status:

Needs review

» Needs work

The last submitted patch, drupal8.help-module.2102453-1.patch, failed testing.

Comment #4

InternetDevels CreditAttribution: InternetDevels commented 5 October 2013 at 11:16

Status:

Needs work

» Needs review

File	Size
drupal-remove_drupal_set_title_in_help_module-2102453-4.patch	851 bytes

Comment #5

5 October 2013 at 12:48

Status:	Needs review	» Needs work
Issue tags:	-WSCCI Conversions

The last submitted patch, drupal-remove_drupal_set_title_in_help_module-2102453-4.patch, failed testing.

Comment #6

disasm CreditAttribution: disasm commented 23 October 2013 at 05:11

Status:	Needs work	» Needs review
Issue tags:		+WSCCI Conversions

#4: drupal-remove_drupal_set_title_in_help_module-2102453-4.patch queued for re-testing.

Comment #7

dawehner

German

CreditAttribution: dawehner commented 24 October 2013 at 23:56

Status:

Needs review

» Needs work

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.php
@@ -106,8 +106,8 @@ public function helpPage($name) {
+      $build['#title'] = $info[$name]['name'];

We should check plain the output, just to be sure and mirror the previous behavior.

Comment #8

ACF CreditAttribution: ACF commented 26 October 2013 at 12:35

Status:	Needs work	» Needs review
Issue tags:		+London_2013_October

File	Size
drupal8.help-module.2102453-8.patch	739 bytes

Added check_plain to the previous patch.

Comment #9

dawehner

German

CreditAttribution: dawehner commented 26 October 2013 at 14:53

Status:

Needs review

» Needs work

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.php
@@ -106,7 +106,7 @@ public function helpPage($name) {
+      $build['#title'] = check_plain($info[$name]['name']);

Let's use String::checkPlain