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

Files: 
CommentFileSizeAuthor
#9 2102449-follow-up-9.patch707 bytesamateescu
PASSED: [[SimpleTest]]: [MySQL] 59,120 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.8 KB
PASSED: [[SimpleTest]]: [MySQL] 58,890 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

All good, except the minor note below:

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.php
@@ -69,10 +69,11 @@ public function buildForm(array $form, array &$form_state, FieldInstanceInterfac
       '%bundle' => $bundles[$entity_type][$bundle]['label'],

Not sure how are we going to handle PASS_THROUGH in D8.

RTBC from my side.

Well, we don't want check_plain here, so we're good. I guess if we need it, we'll have to call check_plain() again.

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.

Status:Closed (fixed)» Needs work

+++ b/core/modules/field_ui/lib/Drupal/field_ui/Form/FieldEditForm.php
@@ -92,8 +92,6 @@ public function buildForm(array $form, array &$form_state, FieldInstanceInterfac
-    drupal_set_title($this->instance->label());
-

Pretty sure this change was wrong. I.e. the title needs to be added back somewhere else. This currently only works because the hook_menu() menu item has a 'title' and if '#title' is not set and the title resolver doesn't return anything either, drupal_get_title() is called which checks the menu item title. I guess simply adding $form['#title'] would be the easiest.

Status:Needs work» Needs review

I don't see what's wrong with it for now - there's a 'title callback' => 'entity_page_label' in hook_menu() - that's just fine no ?
Having to add '#title' everywhere just sounds plain ridiculous.

Well hook_menu() is only for menu links now. That should not be relied on for the actual page output. The fact that it currently works is only an artefact of legacy code. I'm not going to judge whether that is ridiculous or not, but that's simply how things are at the moment. Unless I'm completely missing something, needless to say.

Issue summary:View changes
StatusFileSize
new707 bytes
PASSED: [[SimpleTest]]: [MySQL] 59,120 pass(es).
[ View ]

@tstoeckler is right, we need a $form['#title'] because that hook_menu() entry that has a title callback is going away soon in #2111823: Convert field_ui / Entity local tasks to YAML definitions.

Status:Needs review» Reviewed & tested by the community

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.