Remove drupal_set_title() in PictureMappingFormController controller and update t() with $this->t() in [#2102469]

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
#8	2102469-picture-title-t-8.patch	4.31 KB	vijaycs85
#8
#8	2102469-diff-6-8.txt	4.28 KB	vijaycs85
#6	drupal-picture_remove_drupal_set_title_2102469-6.patch	1.15 KB	ACF
#6
#3	drupal-picture_remove_drupal_set_title_2102469-3.patch	1.37 KB	InternetDevels
#3
#1	drupal-picture_remove_drupal_set_title_2102469.patch	1.15 KB	InternetDevels
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

InternetDevels CreditAttribution: InternetDevels commented 5 October 2013 at 09:00

Status:

Active

» Needs review

File	Size
drupal-picture_remove_drupal_set_title_2102469.patch	1.15 KB

Here is the patch.

Comment #2

5 October 2013 at 10:26

Status:

Needs review

» Needs work

The last submitted patch, drupal-picture_remove_drupal_set_title_2102469.patch, failed testing.

Comment #3

InternetDevels CreditAttribution: InternetDevels commented 5 October 2013 at 13:04

Status:

Needs work

» Needs review

File	Size
drupal-picture_remove_drupal_set_title_2102469-3.patch	1.37 KB

Comment #4

dawehner

German

CreditAttribution: dawehner commented 20 October 2013 at 18:25

Status:

Needs review

» Needs work

+++ b/core/modules/picture/lib/Drupal/picture/PictureMappingFormController.php
@@ -29,11 +30,11 @@ class PictureMappingFormController extends EntityFormController {
+      $form['#title'] = String::checkPlain(t('<em>Duplicate picture mapping</em> @label', array('@label' => $this->entity->label())));
...
+      $form['#title'] = String::checkPlain(t('<em>Edit picture mapping</em> @label', array('@label' => $this->entity->label())));

I can't see a reason why you want to check_plain here, given that it wasn't applied earlier.

Comment #5

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 20 October 2013 at 22:07

@InternetDevels, we should use String::checkPlain(), if 2nd parameter of drupal_set_tiltle() is empty (or CHECK_PLAIN). That's how it works in D7.

Comment #6

ACF CreditAttribution: ACF commented 26 October 2013 at 14:01

Status:

Needs work

» Needs review

File	Size
drupal-picture_remove_drupal_set_title_2102469-6.patch	1.15 KB

Reroll using check_plain.

Comment #7

dawehner

German

CreditAttribution: dawehner commented 26 October 2013 at 15:05

You could also directly use $this->t() instead of t().

Comment #8

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 26 October 2013 at 15:17

Title:	Remove drupal_set_title in picture module controllers	» Remove drupal_set_title in PictureMappingFormController controller and update t() with $this->t() in
Issue tags:		+London_2013_October