Remove drupal_set_title in config_test module controllers [#2102443]

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
#9	drupal8.config-module.2102443-9.patch	1.34 KB	disasm
#9
#9	interdiff.txt	477 bytes	disasm
#7	drupal8.config-module.2102443-7.patch	1.42 KB	disasm
#7
#7	interdiff.txt	716 bytes	disasm
#5	drupal8.config-module.2102443-5.patch	1.11 KB	disasm
#5
#3	2102443-config_test-title-3.patch	1.81 KB	rteijeiro
#3
#1	2102443-config_test-title-1.patch	1.81 KB	vijaycs85
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 4 October 2013 at 10:52

Status:

Active

» Needs review

File	Size
2102443-config_test-title-1.patch	1.81 KB

Initial patch:

Notes:
1. I guess we may need to replace _controller: '\Drupal\config_test\ConfigTestController::edit with _form: '\Drupal\config_test\ConfigTestFormController' as we needed callback just to set the title. thought it is not part of this page.

2. Not sure how are we handling the second param of drupal_set_title(). (i.e. PASS_THROUGH here.)

Comment #2

4 October 2013 at 12:32

Status:

Needs review

» Needs work

The last submitted patch, 2102443-config_test-title-1.patch, failed testing.

Comment #3

rteijeiro CreditAttribution: rteijeiro commented 7 October 2013 at 15:34

Status:

Needs work

» Needs review

File	Size
2102443-config_test-title-3.patch	1.81 KB

Re-rolled.

Comment #4

7 October 2013 at 16:30

Status:

Needs review

» Needs work

The last submitted patch, 2102443-config_test-title-3.patch, failed testing.

Comment #5

disasm CreditAttribution: disasm commented 22 October 2013 at 17:34

Status:

Needs work

» Needs review

File	Size
drupal8.config-module.2102443-5.patch	1.11 KB

Tackling this an different way. title callbacks should be used very sparingly. There's no reason we can't just throw #title on the form itself here. Also, \Drupal::entityManager is ugly, so taking the opportunity to change that to $this->entityManager().

Comment #6

22 October 2013 at 18:26

Status:

Needs review

» Needs work

The last submitted patch, drupal8.config-module.2102443-5.patch, failed testing.

Comment #7

disasm CreditAttribution: disasm commented 22 October 2013 at 19:50

Status:

Needs work

» Needs review

File	Size
interdiff.txt	716 bytes
drupal8.config-module.2102443-7.patch	1.42 KB

Try that again without the missing use flag for ControllerBase.

Comment #8

dawehner

German

CreditAttribution: dawehner commented 23 October 2013 at 13:33

+++ w/core/modules/config/tests/config_test/lib/Drupal/config_test/ConfigTestController.php
@@ -60,3 +62,5 @@ function disable(ConfigTest $config_test) {
+
+

This is one newline too much :P