| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | theme system |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | markup, needs backport to D7 |
Issue Summary
Problem/Motivation
Determination of visibility for a given region's content is done by checking:
<?php
if ($page['region']) {...
?>This results in wrapping divs showing regardless of content existing, which prevents other modules from suppressing various regions through hook_page_alter() w/o resorting to unsetting given regions entirely. (i.e. no using #access = FALSE)
Proposed resolution
Several approaches have been considered:
- Use render() before determining visibility. Patch in #16 implements this solution.
<?php
$region_name = render($page['region']);
if ($region_name) {...
?>
Disadvantages- Confusing for themers (see #1158090: Separate usage of hide(), render(), and other template features from print statements to enable non-developers to create themes )
- From #23:
hide()will not work once the content is rendered, e.g. in:<?php if ($comment_form = render($content['comment_form'])): ?>
<h2 class="title comment-form"><?php print t('Add new comment'); ?></h2>
<?php hide($content['comment_form']['subject']); // This will not work. ?>
<?php print $comment_form; ?>
<?php endif; ?> - See #20 and #32. Many preprocess functions make decisions based on the unrendered state of the variable, which would not be consistent with this approach. The core issue is that each each time an element is themed, only an HTML string is returned and all the info used to render it is discarded.
- Add a helper function.
See #36 and #38. A helper function that checks whether the element should be empty or not (based on the element's values for
#access,#printed, and#children) could be used both in templates and preprocess functions. - Solution 3 (D8 only): Remove all show()/hide()/render() from templates.
Remaining tasks
Determine which solution to use.
Related issues:
- #997408: $tabs is always set
- #1158090: Separate usage of hide(), render(), and other template features from print statements to enable non-developers to create themes
User interface changes
Solution 1 would change tpl files as seen in the proposed code.
API changes
Original report by EclipseGc
I'm filing this against bartik, but in truth, garland, and system module both suffer from the exact same problem.
Currently determination of visibility for a given region's content is done by checking:
<?php
if ($page['region']) {...
?>instead of:
<?php
$region_name = render($page['region']);
if ($region_name) {...
?>This results in wrapping divs showing regardless of content existing, which prevents other modules from suppressing various regions through hook_page_alter() w/o resorting to unsetting given regions entirely. (i.e. no using #access = FALSE)
Eclipse
Comments
#1
better title.
#2
Yes, we need a new pattern here.
Instead of:
<?php if ($page['sidebar_first']): ?><div id="sidebar-first" class="column sidebar"><div class="section">
<?php print render($page['sidebar_first']); ?>
</div></div> <!-- /.section, /#sidebar-first -->
<?php endif; ?>
I suggest we do:
<?php if ($sidebar_first = render($page['sidebar_first'])): ?><div id="sidebar-first" class="column sidebar"><div class="section">
<?php print $sidebar_first; ?>
</div></div> <!-- /.section, /#sidebar-first -->
<?php endif; ?>
#3
Perhaps we should bump this up to critical to get this fixed immediately. Raising to major and shifting to proper queue.
Indeed this is being done in all 4 core page.tpl.php files - system, bartik, garland and seven.
#4
Yeah. I agree it should receive attention before RC1, even if the answer ends up being "won't fix". I don't know if #2 is the best approach though. It makes sense, but is it themer friendly? Any themers want to chime in?
#5
From a keep-it-simple standpoint for inexperienced themers, I'd expect the theme to handle the visibility logic and populate simple $sidebar_first (etc) variables in the appropriate template.php functions, rather than calling render() in a .tpl file.
#6
Working on this now. Will post a patch soon.
#7
Here we go.
Just to make a case for this (as someone who came to Drupal by copying PHP snippets into my custom theme, without knowing a line of PHP):
This only effects places where you need to call render() anyway.
So instead of copying:
if ($page['sidebar_first']) {// ...
print render($page['sidebar_first']);
}
you copy:
if ($sidebar_first = render($page['sidebar_first'])) {// ...
print $sidebar_first;
}
If you don't know PHP, either is cryptic to you, and I don't think you can clearly say the latter is more cryptic. Either way, if you don't know PHP, you probably don't care and just copy&paste anyway.
If you know PHP, either is trivial.
Also from a markup standpoint, this is a bugfix not an aesthetical choice (see also issue category).
Let's see what Dries or webchick say.
#8
+++ modules/comment/comment-wrapper.tpl.php 16 Nov 2010 22:16:47 -0000@@ -37,16 +37,16 @@
+ <?php if ($comments = render($content['comments']) && $node->type != 'forum'): ?>
Bug if I'm not mistaken -- '=' has a lower precedence than '&&' http://php.net/manual/en/language.operators.precedence.php
Needs parens either way.
+++ themes/bartik/templates/comment-wrapper.tpl.php 16 Nov 2010 22:16:49 -0000@@ -37,16 +37,16 @@
+ <?php if ($comments = render($content['comments']) && $node->type != 'forum'): ?>
Same
+++ themes/bartik/templates/page.tpl.php 16 Nov 2010 22:16:50 -0000@@ -209,36 +209,41 @@
+ <?php if ($triptych_first = render($page['triptych_first'])
Same.
+++ themes/bartik/templates/page.tpl.php 16 Nov 2010 22:16:50 -0000@@ -209,36 +209,41 @@
+ <?php if ($footer_firstcolumn = render($page['footer_firstcolumn'])
Same.
If we weren't so late in the D7 cycle, this would be a big -1 from me -- the creation of all of these extra variables like $help just smells bad -- it's not hard to imagine them causing head-scratching when they overwrite other same-named variables. Additionally, using an assignment operator inside if()'s is familiar to us coders, but seems certain to confuse themers who aren't strong on PHP.
Unfortunately, I don't have a better pattern to suggest, and I agree with tstoeckler's evaluation of copy-paste coders not caring what the syntax looks like. Thanks for the patch, tstoeckler.
Powered by Dreditor.
#9
I just re-read and completely agree with comment #5 -- that may be a D8 thing, though.
#10
What was I thinking? Thanks claar!
The latter two examples you give in #8 are correct, though.
I found two other examples where I was doing it wrong, though.
#11
Setting to "needs review".
#12
I think that this approach is (a) a hair too complex for the average themer (b) the least complex option at this late stage.
That said... re the patch in #10
+++ modules/update/update.fetch.inc 16 Nov 2010 23:08:23 -0000@@ -374,6 +374,16 @@ function update_parse_xml($raw_xml) {
+ if (isset($release->files)) {
+ $data['releases'][$version]['files'] = array();
+ foreach ($release->files->children() as $file) {
+ $file_data = array();
+ foreach ($file->children() as $k => $v) {
+ $file_data[$k] = (string) $v;
+ }
+ $data['releases'][$version]['files'][] = $file_data;
+ }
huh? Is this supposed to be part of the current patch? What am I missing
Powered by Dreditor.
#13
Uh?!? No. Way. Those templates have had that markup in them since 2009. They're staying put.
Moving to D8, but this is probably even won't fix. That logic doesn't belong in tpl.php files.
#14
Just for the record: no #12 doesn't belong in there.
#15
Ran into this today in Bartik with $tabs, during a demo.
I added
dpr($variables['tabs']);to bartik_process_page(), and went to the home page:Array(
[#theme] => menu_local_tasks
[#primary] =>
[#secondary] =>
)
The output of render($tabs) is nothing.
In the markup, right after <a id="main-content"></a> was <div class="tabs"></div>
In response to #13, in Garland it was
if ($tabs): print '<ul class="tabs primary">'. $tabs .'</ul></div>'; endif;. There were no renderable arrays, so the if() statement actually did something.I'll roll another patch soon, but #10 is along the right line.
#16
Here we go.
#17
In D6, variables like $tabs contained the actual HTML output, so using
if ($tabs)was valid.With the introduction of renderable arrays, the $tabs variable doesn't contain the HTML output until render() is called.
Calling render() in preprocess/process is not an option, because it removes the ability for a tpl.php to use hide().
Therefore, this patch is only updating the templates to work with renderable arrays. The current behavior is a regression.
In D8 this should probably be revisited, but in the meantime this is the easiest way to fix it for D7.
#18
Patch looks great. You'll have to convince webchick, though, see #13
#19
Tim, Thanks for hopping into this issue. That was kind of my point from the get-go. The logic in the tpl files is insufficient, and regardless of how long it's been there, you CAN and WILL end up with regions that render, but have no content (say, if all the content in a region is set to #access => false). So we need a fix, IN D7. *end rant*
#20
I don't think this patch is going to work, since we have code in a variety of preprocess functions that makes decisions based on the unrendered state of the variable also. For example, in template_preprocess_html():
<?php// Add information about the number of sidebars.
if (!empty($variables['page']['sidebar_first']) && !empty($variables['page']['sidebar_second'])) {
$variables['classes_array'][] = 'two-sidebars';
}
elseif (!empty($variables['page']['sidebar_first'])) {
$variables['classes_array'][] = 'one-sidebar sidebar-first';
}
elseif (!empty($variables['page']['sidebar_second'])) {
$variables['classes_array'][] = 'one-sidebar sidebar-second';
}
else {
$variables['classes_array'][] = 'no-sidebars';
}
?>
I think the checks have to be consistent, or else couldn't we wind up with pages that have the "two-sidebars" class applied to the body, but no trace of two sidebars anywhere in the HTML? That would be even worse than the current situation.
It seems very difficult to fix this in Drupal 7. Maybe the best we could hope for is to add a special case for situations where #access was specifically set to FALSE, and make sure the variable is forced to be empty then. Those are easier to detect since we know exactly what to look for. (It might even be considered a security hardening improvement, to make sure the tpl.php file doesn't have any chance to use variables that the user is explicitly denied access to.)
#21
See also #997408: $tabs is always set.
#22
sub.
#23
Hm. This is a bit mind-boggling. With the suggested approach, you cannot do the following anymore:
<?php if ($comment_form = render($content['comment_form'])): ?><h2 class="title comment-form"><?php print t('Add new comment'); ?></h2>
<?php hide($content['comment_form']['subject']); ?> <---------------------- #FAIL
<?php print $comment_form; ?>
<?php endif; ?>
unless you know that you have to hide() before you render(); i.e., before the entire wrapping condition — and that said, performing the hide() as in this example in case there is no comment form will trigger a PHP notice/warning, since the array key doesn't exist.
#24
How do we prevent empty markup from being printed, without removing the usefulness of render arrays?
#25
@#24: In light of #20/#23:
I think it's pretty clear that, ideally, this patch is the way forward, because no one wants to output empty markup. The question, though, is whether we can get away with potentially breaking existing templates (#23) or theme functions (#20, although that seems to be a bit more difficult, because there's no real workaround, I think) for existing D7 themes.
On the other hand, empty markup can also break layout, styling, etc. in themes. Except that that's been the case for the last year or whenever render arrays got in. And, since it's all templates and theme functions, it's not like people are actually stuck with empty markup on their sites or in their themes.
So in light of #20/#23 I would vote to punt this to D8 unless someone comes up with a way to remove empty markup and retain backwards compatibility.
#26
I'm not too bothered by #23. there are lots of other ways to hide the comment subject. A themer can use existing technique if she really wants.
As for David's #20 - template_preprocess_html() runs after sidebar_first has been rendered so it should be able to check #children and see if it is empty. So, if (!empty($variables['page']['sidebar_first']['#children']). All I did was append a #children to the existing check.
Problem is, #children (and #printed) is never present in $variables during preprocess and is also missing when we do subsequent drupal_render() calls. I added EXTR_REFS to theme_render_template() but there must still be more copies of $elements that are losing this data.
Sorry i can't be more helpful.
#27
Can someone explain the code change that caused this render() call in the template to be necessary?
Adding render() in the template like this makes Drupal templates one step harder to understand. For Drupal 8, where we have a nice new blank slate, can we not work toward a solution that doesn't make templates less readable?
Like Webchick said, the old syntax has been in Drupal a looong time, and it's a shame to change it to something less understandable.
#28
@claar, take a look at core's node.tpl.php around line 100: http://drupalcode.org/viewvc/drupal/drupal/modules/node/node.tpl.php?rev...
Being able to use show() and hide() before render() is awesome.
#29
This issues shows the fundamental flaw in the idea of combining show/hide/render with the desire to be able to conditional flow HTML in the template.
Dealing with wrapper divs that only wrap a single variable can still be done relatively easily. You just have to move the wrapper out of your current template and into the theme hook that the variable is using. e.g. Instead of adding a wrapper div around the page.tpl's $sidebar_first variable, add the wrapper div to region--sidebar-first.tpl.
But dealing with wrapper divs that wrap multiple variables can't be solved that way. The "easiest" solution for d7 is to use the ugly-ish
<?phpif ($foo = render($bar)):
?>
We either need to solve this problem better in D8 or remove all the show/hide/render stuff from templates.
#30
subscribing
#31
subs. Just to say, as a relative newbie to Drupal theming (or at least I was so fairly recently), the addition of render() is very useful indeed but does confuse new people from my experience. I'm talking here of the hobbyist/web-amateur, not experienced professionals who will immediately look up the API.
#32
I looked a little more into this, and especially David's problem from #20.
The core problem is that we have lost all information about what was empty and what was not (i.e. #children) by the time we exit page,tpl.php. So we can't definitively say if any region was empty or not during template_preprocess_html(). While we are in page.tpl.php, $page carefully keeps a history of what stuff was #printed and what each elements contribution to the HTML was. But thats all tossed out when theme_render_template() finishes. Each time we theme an element, we discard this important info and only return a string of HTML. I would love for each element's #children and #printed to persist inside of drupal_render(). Though we might have to be satisfied with a #empty boolean instead of #children in order to conserve memory ... I don't immediately know how to accomplish this persistence though.
#33
Re #31: Thanks for your feedback. We do need to get a better handle on how many themers are finding difficulty with render(), and whether the solution is documentation, rethinking some architecture, or something else. Please join that discussion on #1158090: Separate usage of hide(), render(), and other template features from print statements to enable non-developers to create themes , especially if you have ideas on how we can move that discussion from just anecdotal guesses to real "themer experience" data.
If a significant change gets committed in that issue, that may change this issue's direction. But in the meantime, let's keep this issue focused not on whether render() in templates is in itself confusing, but whether something along the lines of #16 makes sense, given the problem in #20, summarized well in #25, and getting dug into a little more in #32.
#34
#35
subscribing
#36
What's wrong with using a helper function? Was this considered already?
This function might need more conditions but you get the idea.
<?php
/**
* Check if the render array element has content.
*/
function has($element) {
if (isset($element['#access']) && !$element['#access']) {
$element = FALSE;
}
elseif (!empty($element['#printed']) && isset($element['#children'])) {
$element = $element['#children'];
}
return !empty($element);
}
?>
<?php// Add information about the number of sidebars.
if (has($variables['page']['sidebar_first']) && has($variables['page']['sidebar_second'])) {
$variables['classes_array'][] = 'two-sidebars';
}
elseif (has($variables['page']['sidebar_first'])) {
$variables['classes_array'][] = 'one-sidebar sidebar-first';
}
elseif (has($variables['page']['sidebar_second'])) {
$variables['classes_array'][] = 'one-sidebar sidebar-second';
}
else {
$variables['classes_array'][] = 'no-sidebars';
}
?>
<?php if (has($content['comment_form'])): ?><h2 class="title comment-form"><?php print t('Add new comment'); ?></h2>
<?php hide($content['comment_form']['subject']); ?>
<?php print render($content['comment_form']); ?>
<?php endif; ?>
This doesn't complicate things when you compare it with !empty(). It's actually easier to read IMO.
#37
@dvessel - did you actually run that code? when i looked into the problem described in #20, #children has vanished by the time we enter into template_preprocess_html(). the reason why is #32.
#38
No, but maintaining persistence would need to happen in two places.
theme_render_template is the easy one and doesn't maintain references.
<?phpextract($variables, EXTR_SKIP); // Extract the variables to a local namespace
?>
That can be fixed with the
EXTR_REFS + EXTR_SKIPflags.Not sure how theme() will behave when the "$variables" parameter is referenced and that may be trickier.
#39
Subscrizzle
#40
subscribing
#41
...subscribing.
#42
subscribing
#43
Tagging.
#44
#7: 953034-template-files-empty-render.patch queued for re-testing.
#45
Setting back to NW (patches are in old format). First summary added by e-anima.
#46
Corrected and clarified the summary a bit.
#47
Hey all, can we please figure out a way forward for this soon? We're running into issues with this while converting templates to HTML5. I've been using the approach Damien recommends in #2, but don't want to implement that across all these templates until a decision has been reached here. We've only really attempted to address Drupal 7 here. What's the plan for Drupal 8?
#48
IMO, #2 is the solution until someone has a working alternative. I'd say that improvements to templates should not wait on this issue.
#49
Alright, thanks Moshe. :)
Anyone else have an issue with this? If everyone is alright with #2 then perhaps we should fix it all in one patch here.
#50
#2 is basically fine for D8, as long as the markup maintainers are fine with more PHP logic being introduced into template files. The only complaint raised against it is #23, which is a bit of an edge case.
We need another, less every-template-changes-as-a-result solution for D7 though.
#51
I still think we should get all the render calls out of all of the template files in core.
Let's move them into preprocess, and then we can have clean and simple logic only in the template files.
function whatever_preprocess_page(&$variables) {$variables['sidebar_first'] = render($page['sidebar_first']);
}
and then the templates are NOT SCARY anymore and any HTML person can understand them
<?php if ($sidebar_first): ?><div id="sidebar-first" class="column sidebar"><div class="section">
<?php print $sidebar_first; ?>
</div></div> <!-- /.section, /#sidebar-first -->
<?php endif; ?>
Don't get me wrong, I like render. But it does not belong in core template files. It's an advanced tool, and advanced people will not be using core themes anyway. Beginners will be looking at core themes, and these files need to make sense to them.
#52
Here's a patch that updates all of bartik's template files. Take a look and let me know what you think.
#53
This is the same as the patch in #52 with all the whitespace issues removed
#54
Damn, #52 looks sexy. Certainly makes our template files look a lot more like standard (and Googleable) HTML + PHP again, which is great for "normal" people who just want to take a theme and tweak it. It does mean a bit more work for theme developers, though, but I think those folks are largely used to muddling around in template.php and preprocess functions already.
Obviously we can't backport that approach to D7 unforutnately, though. :(
#55
Here's #53 again with .patch, for testbot.
I think we should go this route for D8. I'll write a patch for D7 that follows #2 if we can't find a more elegant solution.
#56
I like this approach. Much better for #1158090: Separate usage of hide(), render(), and other template features from print statements to enable non-developers to create themes .
Note that #55 has some trailing whitespace and some inline comments that should probably be wrapped at 80 chars, so we'll want to clean that up.
#57
This is a re-roll of #55 that I believe fixes the whitespace issues.
#58
The justification given so far for removing the render() calls is that they are SCARY. Lets balance that against the benefits provided by render() in templates. I think these benefits are pretty compelling:
#59
Basically we have to weigh Moshe's very legitimate set of objections against the simple problem that we can't run a clean if statement against un-rendered arrays. That's the bottom line. Our two primary objections are summed up thus:
Someone's got to give here... personally I favor the ugly fix cause it's not THAT complicated, and designers are used to copying and pasting stuff they know works.
#60
Responses to Moshe's
objectionslist of benefits in #58:I don't think moving render() to preprocess changes this principle, but correct me if I am wrong.
render() doesn't guarantee that people aren't going to still write bad code. If people don't understand how to sanitize their data, that is not our problem. Giving them render() is like giving them a slap-chop instead of teaching them how to use a knife.
I don't see how there is extra work? Either we are rendering in a tpl and checking for an empty string or we are rendering in template.php and then checking for an empty string in the tpl. The argument that "folks will often print theme everything into $content AND theme the bits" with regards to performance follows the same response I had for #2: If people don't understand how to render variables separately, that is not our problem. It *is* our problem to have an inconsistent theming API, imho. Also see my response to #4.
This argument has always been troublesome to me. Anyone who is serious about theming is going to have to learn preprocess functions someday; It's like wanting to be a race car driver without learning how to drive manual transmission. As someone who has given several Drupalcon sessions about preprocessor functions, I have had direct feedback from attendees about how useful the concepts are and how attendees felt like they now "understand" theming in Drupal. We should stop fooling ourselves that themers and designers aren't smart enough to write PHP and dive into template.php -- it is inevitable that they will have to enter this world someday if they are going to be writing custom themes in Drupal.
Saying "you don't have to use render()" is an argument for having render() in tpls? Do I misunderstand? :)
#61
I do not think we should use render() in Bartik's template files. I believe the opposite of Moshe's #5.
Bartik is the default theme for Drupal, and that means that people who don't know any better are going to be changing it directly, copying it to make their own theme, or doing who-knows-what else do it - as beginners will do. These are people who will not understand how to properly use render(). These template files are the ones that need to be the most inviting - because this is where people will start.
Sure - let's leave render in stark. We can assume that people who will be starting from scratch (or at least think they want to) can figure it out. But in Bartik? no way.
Moving the render calls to preprocess in Bartik solves this issue, and is a start to the bigger problem in #1158090: Separate usage of hide(), render(), and other template features from print statements to enable non-developers to create themes
#62
Hm. I wonder if a lot of Moshe's concerns could be mitigated by changing:
+ <?php print $content; ?>to
+ <?php print render($content); ?>Just $content (and things inside $content, like $content['links']), no other template variables. That would still remarkably clean up the template output while also exposing to themers to the power of the render API, thus hopefully alleviating their tendency to do a D6-like "blow away this crappy $content variable and start printing raw field values directly" behaviour that I think Moshe is concerned about.
One thing:
"As someone who has given several Drupalcon sessions about preprocessor functions, I have had direct feedback from attendees about how useful the concepts are and how attendees felt like they now "understand" theming in Drupal."
Yeeeeahhh. That's actually a problem that sort of proves Moshe's point. :) Not everyone has the ability to go to DrupalCon and hear the almighty Carl Wiedemann explain preprocess functions to them. :) The .tpl.php file is the more natural workspace of a novice-level themer/designer, as evidenced by the unholy things you will find in there with themes coded by novices (SQL queries, 70,000-line functions, and the like). Exposing hints that there are more flexibe ways to render content in the tpl.php file itself will likely lead to more people making their way to template.php without "outside" help.
#63
Hm. This is an interesting idea.
I still think that calling functions like hide() are going to scare people away - and I don't want that in Bartik's tpl.php files - but maybe there's a way we can render($content) after rendering links and other parts of $content in preprocess.
I'd be willing to compromise on something like this.
#64
I am no fan of render(), but "pre-render everything we may possibly need just in case" is exactly the core bug we need to get away from.
c4rl's point is also valid. At some point we have to just accept that theming a complex system like Drupal is complex; complex theming cannot be done by a novice whose only experience is Dreamweaver.
We already have a system where a non-PHP dev is hamstrung when trying to theme stuff. Every single version of Drupal since I've gotten involved has tried to make theming friendlier toward someone who doesn't know what they're doing, and every version it's gotten more complex and hasn't fixed that problem. (I'd argue Drupal 6 actually had the best balance there.)
Perhaps instead of putting our energy into "make it possible for someone to theme without understanding WTF is going on", we should put it into "make it easier for someone to *learn* (not understand intuitively, learn) WTF is going on so they can be useful." And if that means that anything more complex than pure-CSS you need to learn at least some PHP for... Then we're just being honest about what is, arguably, already the situation.
#65
I have not tested for possible side effects but if we must render before the template, using the second phase variable process functions will still allow some flexibility. It will allow modules and themes to preprocess specific fields before render() is called. I would test it myself but we're experiencing a major power outage.
#66
Well, now that I have finished swooning ;) I will remark that part of the issue here is a documentation/learning-curve issue. Not only do we need a consistent API, but we need to explain the API.
A comment and patch on a related issue by David_Rothstein beings to address this question. (#1158090: Separate usage of hide(), render(), and other template features from print statements to enable non-developers to create themes )
#67
In addition to Moshe's concerns, please also consider the subtheme issue. Copying from #1158090-92: Separate usage of hide(), render(), and other template features from print statements to enable non-developers to create themes :
If we believe that bartik is not a good base theme, then ok, let's put render() in process(), and document somewhere why we don't think it's a good base theme.
If we want to move render() out of templates and also support subtheming, then we need a new type of callback that runs after process(), and that only runs for the concrete theme and not for base themes.
#68
Wow. That's a prickly one, indicated in #67. Indeed subtheming by definition requires the base theme's preprocessor to run.
So render(), by design, isn't *currently* intended for preprocessors, but rather for templates, and the subtheming issue is thus a consequence.
I don't want to throw off the conversation too much, but does this beg the question whether render() should change (and if so, how can render() change) to be used in preprocessors?
/me ducks :)
#69
I believe this isn't correct. Process functions always come after preprocess no matter which module or theme implements them.
To be clear, I'd rather have consistent use of render(). Rendering in some_process_foo() will inevitably snowball into more inconsistent behavior.
#70
A few Centauri Ducats:
Ideas:
Putting 1 and 2 together gives you:
<?php if (drupal_empty($monkey)): ?><div class="monkey"><?php output($monkey); ?></div>
<?php endif; ?>
#71
Once we get this right for D8, we can backport it in D7 by allowing D7 to check arrays for visibility both ways. This will offer some sort of early adoption of the new "proper" way by themers in D7 while still allowing themes crafted the old way to work (or rather not cause them to break). The old way can then be completely deprecated in D8.
Am I making any sense?
#72
@klonos: Ok, but I recommend we completely ignore D7 in the current discussion for now.
Based on various feedback above I'm changing this back to NW.
FYI there's an issue to review another theming engine with a view to completely replacing the PHPTemplate in D8: #1441320: Evaluate Twig template engine for Drupal core (as an alternative for PHPTemplate and possibly even theme functions)
#73
For D7, I'm pretty sure Damien's original solution in #2 works fine for any theme that wants to adopt it.
But as for the core themes, I'm not sure we'll ever have a backportable fix here? (By definition, fixing this means changing the HTML markup in some cases...)
#74
#1552464: Empty tabs <div> rendered, but <?php if ($tabs): ?> is always TRUE marked as duplicate
#75
is there now a solution for the never empty tabs?
it costs me a hour, to find out that this is a bug in D7 ...
symfony and twig are coming - my only comfort
i'm sorry but drupal7 is like vista - a monster ...
#76
@sachbearbeiter, posts like that are unlikely to make anyone want to help you resolve the issue.
The main problem in this issue is finding a solution which is backportable to Drupal 7 -- we have to weigh carefully changing the output, because this will very easily break existing contributed and custom themes. See: http://drupal.org/node/1527558
See comment #2 for a workaround you can use in D7.
#77
thanks ...