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()

CommentFileSizeAuthor
#8 2102469-picture-title-t-8.patch4.31 KBvijaycs85
#8 2102469-diff-6-8.txt4.28 KBvijaycs85
#6 drupal-picture_remove_drupal_set_title_2102469-6.patch1.15 KBACF
#3 drupal-picture_remove_drupal_set_title_2102469-3.patch1.37 KBinternetdevels
#1 drupal-picture_remove_drupal_set_title_2102469.patch1.15 KBinternetdevels

Comments

internetdevels’s picture

internetdevels commented
Status: Active » Needs review
StatusFileSize
new drupal-picture_remove_drupal_set_title_2102469.patch1.15 KB

Here is the patch.

Status: Needs review » Needs work

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

internetdevels’s picture

internetdevels commented
Status: Needs work » Needs review
StatusFileSize
new drupal-picture_remove_drupal_set_title_2102469-3.patch1.37 KB
dawehner’s picture

dawehner
Primary language German
commented
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.

vijaycs85’s picture

vijaycs85
Location London, UK
commented

@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.

ACF’s picture

ACF commented
Status: Needs work » Needs review
StatusFileSize
new drupal-picture_remove_drupal_set_title_2102469-6.patch1.15 KB

Reroll using check_plain.

dawehner’s picture

dawehner
Primary language German
commented

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

vijaycs85’s picture

vijaycs85
Location London, UK
commented
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
StatusFileSize
new 2102469-diff-6-8.txt4.28 KB
new 2102469-picture-title-t-8.patch4.31 KB

Here is the updated title and patch that covers it.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Perfect!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Remove drupal_set_title in PictureMappingFormController controller and update t() with $this->t() in » Remove drupal_set_title() in PictureMappingFormController controller and update t() with $this->t() in

Fix title.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Component: picture.module » responsive_image.module
Issue summary: View changes