empty node links array causes problems in xtemplate theme

This doesn't happen in the original bluemarine, line 149:

<?php print $link ? "» ".$link : ""; ?>

IMO it should be fixed in xtemplate.xtmpl, line 63:

<div class="links">» ‹links›</div>

but I don't have enough experience with XTemplate to fix it.

This is a theme-specific problem, as noted. Changing away from comment.module to "Theme system" and taking ownership. I'll patch this soon.

Dries - should we create some project areas/components for each of the core themes? It would make tracking bugs like this easier. Or even have a completely seperate Project called Themes?

Not sure but we might want to fix this in the comment module; for example, by setting some non-clickable text. Like that we wouldn't have to worry about this in the themes.

(I'll update the project areas during the weekend.)

Updating title for charset change. ing" alt="Smil
ing" />

Patch for xtemplate in contributions/for_review/

On Thursday, June 12, 2003 1:53 AM [GMT+1=CET],
Al Maw <drupal-devel at drupal.org> wrote:

> 12/06/2003 - 00:53 : Al Maw
>
Patch for xtemplate in contributions/for_review/

i sent a patch for this earlier [1] which fixes this by introducing a
conditional block for the &raquo; / links /in the template/:

<!-- BEGIN: links --><div class="links">&raquo;
{links}</div><!-- END:
links -->

ie. if there are links, render the "links" block, including the
&
raquo; - if there aren't, don't render the it. this if/then is done in
the template code (xtemplate.theme:

&nbsp;&nbsp;if ($links) {<br />
&nbsp;&nbsp;&nbsp;&nbsp;$this-&gt;template-&gt;assign(&quot;links&quot;, $links);<br />
&nbsp;&nbsp;&nbsp;&nbsp;$this-&gt;template-&gt;parse(&quot;comment.links&quot;);<br />
&nbsp;&nbsp;}<br />

).

the advantage of this over your approach is that the &raquo; is
"
defined" in the template - if the template designer wanted to replace
it with, lets say, an icon, she could just do so in the template. with
your approach, she had to touch the code. the &raquo; quite clearly is
a
design element, so i'm quite convinced that my approach is the better.
besides, if there are no links, you render an superfluous empty <div>,
while i completely suppress it.

my patch also simplified the template by mergin the "comment_new" and
"
comment_old" blocks, which only differ in a <span class="new"> for
unread comments, by introducing a similar conditional as above in
xtemplate.xtmpl.

unfortunately, dries didn't like this:

> I didn't want to optimize the theme because that makes it is easier to
>
reuse. People don't want to mess with the theme file; they want to
>
touch the xtmpl and css file only.

i think this is completely wrong. because

a) duplicating code (html) in the template is error prone. if you want
to change the comment layout, you have to do the same changes at 2
places ("comment_new" and "comment_old"). imagine having
a template
block (<!-- BEGIN: block -->...<!-- END: block -->) spanning 50
lines.
now if you wanted the first line of that block to be bold sometimes and
sometimes plain - you wouldn't duplicate the whole block to achieve
this, would you?

b) having "conditionals" in the template doesn't make it harder
to
reuse. contrarily, it gives designers more power /without having to
touch the code/. of course, the template code has to provide (and, well,
document ...) these conditionals. thats why /i/ mess with the code,
provide it, and sent this patch.

seems to me that the idea of templating, ie. doing /the complete/
markup, /including simple conditional and loop evaluation/, in the
template, and letting the template code provide the "hooks" for this,
hasn't really been understood / accepted ...

comments?

[1] http://lists.drupal.org/pipermail/drupal-devel/2003-May/025100.html

not fixed - see http://drupal.org/comment/reply/1998/3225 for an example.

i think dries' solution - showing a non-clickable example "link" when previewing comments - isn't the proper way to fix this. instead, i think, comment preview should, just as node preview, show all the links the final comment would show. the same - doing it as the node does - should be done to avoid the » when there are no links: let the /theme/ (theme->comment()) check the links and not render anything if there aren't any.

The empty links problem doesn't just manifest on Preview. It can happen for anonymous users reading brief nodes. See this NSHP page for example.

We have to handle empty links correctly, not forciblly add links during node preview.

it gets worse. on the home page, if you display a node with no links, the links from the prior node cary down to the empty node. this looks real bad. it happenned to me on a live site and i had to enable the stupid node counters in order to fix it fast.

whether we go with Ax's suggetsion or something else, we do have to fix this. it most certainly is a bug.

Moshe wrote: if you display a node with no links, the links from the prior node cary down to the empty node.

I wonder how this could happen because the node template is being reset after it has been emitted.

That aside, I'm OK with Ax's suggestion as long the code is nice and clean. I don't want to complicate theme developement more than necessary.

should be fixed with yesterdays patches [#]_ / commits [#]_[#]_. marking so.

.. [#] http://kollm.org/cgi-bin/cvsmonitor/cvsmonitor.pl?cmd=viewBrowseFile&fil...
.. [#] http://kollm.org/cgi-bin/cvsmonitor/cvsmonitor.pl?cmd=viewBrowseChangeSe...
.. [#] http://kollm.org/cgi-bin/cvsmonitor/cvsmonitor.pl?cmd=viewBrowseChangeSe...