Remove drupal_set_title in views module controllers (last instance of drupal_set_title() in modules)

1. +++ b/core/modules/views/includes/ajax.inc
   @@ -35,7 +35,6 @@ function views_ajax_form_wrapper($form_id, &$form_state) {
      if (empty($form_state['ajax']) && !empty($form_state['title'])) {
   -    drupal_set_title($form_state['title']);
        drupal_add_css(drupal_get_path('module', 'views_ui') . '/css/views_ui.admin.css');
      }
    
   @@ -64,11 +63,6 @@ function views_ajax_form_wrapper($form_id, &$form_state) {
   
   @@ -64,11 +63,6 @@ function views_ajax_form_wrapper($form_id, &$form_state) {
        return $response;
      }
    
   -  // These forms have the title built in, so set the title here:
   -  if (empty($form_state['ajax']) && !empty($form_state['title'])) {
   -    drupal_set_title($form_state['title']);
   -  }
   -
   

   Have you manually tested that this actually works? I ran into issues with titles on dialogs today.

2. +++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Page.php
   @@ -91,7 +91,7 @@ public function execute() {
   +    $render['#title'] = filter_xss_admin($this->view->getTitle());
   

   I would love to get rid of this hunk as this is already heavily rewritten on #2068471: Normalize Controller/View-listener behavior with a Page object

.

.

I applied the patch, created node, edited the node and viewed admin content page. The title seems to appear fine in all the pages.

Steps:
1. Did a git pull to get the latest code for 8.x
2. patch -p1 < drupal-remove_drupal_set_title_in_views_module-2102489-1.patch , at the Drupal root folder
3. Output:
patching file core/modules/views/includes/ajax.inc
Hunk #2 succeeded at 65 (offset 2 lines).
patching file core/modules/views/lib/Drupal/views/Plugin/views/display/Page.php

Note: I am a newbie. Please do let me know any additional testing needs to be done.

Note: I am a newbie. Please do let me know any additional testing needs to be done.

Try to have a look at the different dialogs of views. So go to admin/structure/views and edit one of the existing ones.
Click on some of the edit options on there and have a look whether the expected title appears. You could test for example test with and without the patch.

Reviewed and Tested

The title for content page is same before and after the patch applied. Checked all other views and title is coming same before and after applying the patch.

Reviewed and tested.

Sorry, that's not quite the manual testing that dawehner was asking for. Rather than testing titles on a view itself, he was referring to testing titles on the dialogs within the Views admin UI.

However, I just did that and it seems to work well. The titles of the all the Views admin dialogs are coming through in my testing. Yay for fewer calls to drupal_set_title()! :)

I couldn't quite parse from "I would love to get rid of this hunk as this is already heavily rewritten on #2068471: Normalize Controller/View-listener behavior with a Page object" if it was meant that Daniel didn't want that part of the patch committed, or whether he wanted that to happen because it would make that issue easier. But in case it was the former, #2068471: Normalize Controller/View-listener behavior with a Page object is currently set to needs work anyway, so hopefully it doesn't make it too annoying if I commit this one in the meantime. If so, let me know, and I can temporarily revert it, too.

Committed and pushed to 8.x. Thanks!

Removed a drupal_set_title from Views UI. The title seems to be handled on line 368.

Ha, we have test coverage!

Could this possibly be a solution?

15: drupal-remove-drupal_set_title-in-views-2102489-15.patch queued for re-testing.

15: drupal-remove-drupal_set_title-in-views-2102489-15.patch queued for re-testing.

Seems OK, bot likes, and this is our last drupal_set_title(). Yay!

Awesome!

Since Daniel caught a snafu with this last time, assigning to him for final review.

Also, since this is the last thing blocking #1830588: [META] remove drupal_set_title() and drupal_get_title() (which is critical + a beta blocker), also escalating this to major.

Clicked a bit through the UI and all titles still appears as expected.

Sadly this will conflict with #2146473: Move views/includes/ajax.inc to a class in views_ui module which is RTBC since quite some time.

Cool, thanks for checking. And indeed, sorry about that. I've added that to my list of things to get in right after alpha8.

Committed and pushed to 8.x. Thanks!

Oh, boo. :( This was not the last instance of drupal_set_title() at all. :( There are tons of them in includes and whatnot. Left a comment at #1830588: [META] remove drupal_set_title() and drupal_get_title().