Hide empty page title pane when wrapper tag is used

I had the same problem, but came up with a slightly different fix, as I don't see any reason to create the token if the title isn't set.

#5 patched worked for me

#5 looks good, applies, does the job. Dig the logic behind it.

This seems to break page_manager titles.

the check to drupal_get_title() is too early, page manager calls drupal_set_title *after* this check has run.

Yup my CI build flagged an error when creating a new draft and checking for the existence of the title on the new page save. The title was not present at all.

Really glad this was reported and have confirmation, this might effect a few people.

Just providing a patch to revert this commit for drush make purposes on top of latest stable (1.9)

Moving this to the v7.x-1.10 release plan.

Ugh. Back to the drawing board for v7.x-1.10.

Apologies, looks like I was wrong and the ultimate culprit (at least for me) was #2437773: Attached CSS and JS files are not loaded where they changed logic to do with the node_view.inc.

https://www.drupal.org/node/2437773#comment-10240035

Reverting one commit before resolved my issue.

Yep, I'll write a test

@jwilson3 @sylus, #5 holding page title to render while #13 resolves this. cause

if (!drupal_get_title()) {
    return;
  }

wont have any title to show because it got called before drupal_set_title

Patch that reverts for those using a make file etc

I also stumbled upon this issue where my page titles from ctools content types was used for title and no longer showed. Using patch in #20 until new release is out.

I can confirm this is an issue when creating custom Panels pages.

There is a further issue with this commit in the case where a title override is specified for a page that has no menu item. In such a case, the call to drupal_get_title() returns NULL because the page title has not yet been set (#11) and no menu item exists.

This slight modification will cause the function to return without setting a title as intended by the original commit -- except when a title override is specified.

if (!drupal_get_title() && $conf['override_title'] != 1) {
    return;
} 

See attached. It creates a page "/test" that demonstrates the problem. If you view the page using CTools 7.x-1.9, the title will not be displayed. Add a menu item pointing to /test and the title will be displayed (because drupal_get_title() will return TRUE). Remove the menu item and the title will not be displayed. Remove these lines:

if (!drupal_get_title()) {
    return;
}

from ctools/modules/plugins/content_types/page/page_title.inc (line 32) and the title will be displayed whether the menu item is present or not. Changing the lines to

if (!drupal_get_title() && $conf['override_title'] != 1) {
    return;
  }

allows the drupal_get_title() test to remain in place while taking into account the possibility of a title override being specified.

Patch in #23 works for me. I had a panel which contained only a custom text pane and a title pane, and the title pane didn't render without this change.

Patch 23 looks pretty good to me, with an exception of this:

+++ b/plugins/content_types/page/page_title.inc
@@ -29,7 +29,7 @@ $plugin = array(
+  if (!drupal_get_title() && $conf['override_title'] != 1) {

While the example plugin modules show override_title to be 0 or 1, the rest of the ctools code has this as a boolean. Lets stay consistent with the rest of the module plugins.

/me makes note to update example plugin code....

This didn't get added to 7.x-1.10.

Confirming the patch in #23 solves the problem. Attached patch changes 1 to TRUE as requested.

Still works for me. Thanks!

I can confirm that this still cause issue with Panels pages (as described in #11).
Patch #29 and #23 does not solve the problem.

Here is an updated patch with proper fix.

jibran, the the patch in #32 does a drupal_get_title() in the same place as the patch that roland.molnar indicated fails with Panels pages, presumably because drupal_set_title() hasn't been called yet (as in #11).

Can you confirm whether you have tested the fix in #32 with the scenario mentioned in #11?

@rivimey drupal_set_title() is called in drupal_get_title() so calling it in the sampe place is alright. I added the second condtion to the statement $conf['override_title'] === PANELS_TITLE_NONE which means if title is not set and page is configured not to show the title then bailout. WIth the addition of $conf['override_title'] === PANELS_TITLE_NONE we made sure that doesn't matter title is set or not if page is configured to show the title then go ahead and set the title.

I tested the exact scenario mentioned in #11. I was updating the same site on which @larowlan encountered the problem. @larowlan RTBCed it cause he tested that fix and tests are passing on a client's project.

I'm not sure how to write tests about it in ctools but I can confirm that I had some Behat test failing on client's project because the fix was not right and they are passing now after the patch.

@jibran, many thanks for your explanation earlier, which was helpful.

i thought the problem with drupal_set_title() was not that it was not called but that it wasn't called with the correct title string : as I am sure you realize, drupal_set_title() just stores a string provided statically (ugh!). It would be good if @larowlan could confirm?

I am slightly wary about the new constants. As the names suggest, the PANELS_TITLE_NONE et al are defined in panels.module. I have checked and a ctools installation with panels not present doesn't crash and burn always, but because it's all about plugins and config it is very possible I haven't done the right thing to invoke it. Is there anything that prevents this file being read by php unless panels.module has already been loaded?

[Note: ordinarily I would be very happy to see named constants replacing magic numbers, but aversion to breaking things is even higher priority :-) ]

It would be good if @larowlan could confirm?

I'll ping him.

As the names suggest, the PANELS_TITLE_NONE et al are defined in panels.module.

Good catch I'll fix this up.

I can't remember...this issue has been around so long. I'll chat with Jibran about it in person this week.

@larowlan, @jibran many thanks.

Here is a new patch.

Fixed a doc issue.

Yeah, from comment #11, it wasn't that the wrong title was used, it was

the check to drupal_get_title() is too early, page manager calls drupal_set_title *after* this check has run.

So the patch at #43 is good in my book.

We should create a follow up for moving those panels constants into ctools and deprecating the panels ones.

I think some additional +1s for the latest patch would be good to see, but as far as I can tell this is looking good.

@larowlan would you be able to create the new Issue for

moving those panels constants into ctools and deprecating the panels ones.

although I suspect it won't be quite that simple :)

I think some additional +1s for the latest patch would be good to see

The patch in #43 is working for me, thanks! :) +1

The patch in #43 is working great for me as well. Thanks for all the hard work everyone!

This looks okay to me in theory, but lets add an isset or empty around the override_title key just in case.

This wasn't finished in time so hopefully it'll be finished in time for 1.13.

Very well, this patch adds an isset().

Looks fine to me. :-) Thanks Jen.

Merged in #43. Thanks!