Textpattern CMS support forum
You are not logged in. Register | Login | Help
- Topics: Active | Unanswered
Inconsistent Behavior: txp:linklist vs txp:file_download_list
I’ve found that txp:linklist and txp:file_download_list do not behave the same when given an empty “wraptag” attribute. I’m trying to combine the output of txp:linklist and txp:file_download_list into a single list, like so:
<ul>
<txp:linklist wraptag="" break="li" />
<txp:file_download_list wraptag="" break="li" />
</ul>txp:linklist wraps the links with <li> but txp:file_download_list does not. A look at the txp source confirms the different behavior. Is there good reason for this? Or would it be safe to change txp:file_download_list to have the same behavior as txp:linklist?
Seems like a bug and it would be great if it could be fixed in 4.0.8. ;)
Last edited by artagesw (2008-12-31 08:14:47)
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
Submit a patch :)
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
I’d be happy to submit a patch. But I’d like a dev to look at the existing code first and compare these two tag handlers. txp:file_download_list is making a special case of ul and ol tags and there is no comment in the code to explain why it might be doing that. There may be a good reason for it that I am unaware of.
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
The break attribute is ignored if the wraptag attribute isn’t set and for anything other than ol/ul it assumes the break tag doesn’t surround the listed item. That doesn’t seem right, and like I am wondering if there is a special case where this is useful.
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
I agree it doesn’t sound right. The txp:linklist tag works fine without making that assumption. In fact, it is more powerful to not require a wraptag, because that allows combination of tags such as what I am trying to do. I’ll patch my installation to verify that it behaves as expected with no strange side effects.
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
OK, here’s the patch. Working fine for me.
diff -NBaur textpattern.cur/textpattern/publish/taghandlers.php textpattern.new/textpattern/publish/taghandlers.php
--- textpattern.cur/textpattern/publish/taghandlers.php 2008-12-31 18:09:48.000000000 -0800
+++ textpattern.new/textpattern/publish/taghandlers.php 2009-01-01 17:02:53.000000000 -0800
@@ -3286,12 +3286,7 @@
if ($out)
{
- if ($wraptag == 'ul' or $wraptag == 'ol')
- {
- return doLabel($label, $labeltag).doWrap($out, $wraptag, $break, $class);
- }
-
- return ($wraptag) ? tag(join($break, $out), $wraptag) : join(n, $out);
+ return doLabel($label, $labeltag).doWrap($out, $wraptag, $break, $class);
}
}
return '';Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
On a similar subject, I have been finding that if wraptag is ‘ul’ or ‘ol’ and break is empty, doWrap is not changing the break to ‘li’ as it should. This is true for me both on my local and live setups (both running TXP 4.0.8 and php 5.2.6). I am working on a plugin and tried giving break a default value of NULL and that didn’t work either.
I tested this with <txp:linklist wraptag="ul" /> and found the same thing: no <li> tags are added.
Going through doWrap(), I see that line 2627 catches the empty break attribute so that you never get to line 2638, where doWrap() tests for the empty break along with wraptag equal to ‘ul’ or ‘ol’.
Changing the match pattern on line 2627 from Ack, does indeed cause problems.'/^\w+$/' to '/^\w*$/' fixes this. I don’t know if it causes other problems, but I don’t see how it would.
Last edited by jsoo (2009-01-04 20:09:20)
Code is topiary
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
That’s a good thing, actually. It allows you to add the <li> in the list form.
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
OK, thanks
So, no need for lines 2638-2641 in taghandlers.php?
Last edited by jsoo (2009-01-04 20:12:15)
Code is topiary
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
I think wraptag and break should be explicitly specified if you want to override them.
Assumptia is the mother of all screwups, so this is one place where TXP shouldn’t assume too much, IMO.
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
Right, I’m just saying that on that basis there’s a vestigial block in doWrap() (lines 2638-2641). It can never be reached, because the block above it catches all cases where break is empty.
Code is topiary
Offline
Re: Inconsistent Behavior: txp:linklist vs txp:file_download_list
You’re right about that block of code never being reached. That is a bug. I think it should be removed, but that won’t happen in TXP 4.0.8. Thanks for reporting it though!
Offline