<img> inside <go> appears to break it.
raintonr - December 6, 2006 - 04:13
| Project: | Go - url redirects |
| Version: | 5.x-1.3 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | spiderman |
| Status: | closed |
Description
Trying to link images doesn't seem to work:
<go href="http://some.site.net/"><img src="/system/files/blah-di-blah"></go>The tags are left alone, ie. not translated to <a>
#1
Fast forward 9 months and a new branch for D5.x release and I can confirm this bug is still present.
Anybody wants to step in with a patch?
#2
actually, i don't think this is really a bug- i found on my system that i could reproduce the "bug" until i realized that tags were being stripped by the default Filtered HTML settings. once i added to the admin/settings/filters/1/configure page (Allowed HTML tags), my test node with a tagset worked fine.
incidentally, this surprised me, because i had thought there *was* a bug in the regex that grabs the args and the text of the link, around line 163:
<?php
diff -ur gotwo/gotwo.module gotwo.patched/gotwo.module
--- gotwo/gotwo.module 2007-06-19 16:03:00.000000000 -0400
+++ gotwo.patched/gotwo.module 2007-10-15 19:44:59.000000000 -0400
@@ -160,7 +160,7 @@
return $text;
case "process":
- return preg_replace('#<go\s([^>]*)>([^<]*)</go>#ise', '__gotwo_filter("\1", "\2")', $text);
+ return preg_replace('#<go\s([^>]*?)>(.+?)</go>#ise', '__gotwo_filter("\1", "\2")', $text);
default:
return $text;
?>
however, when i removed my patch, the preg_replace appears to continue to work, even tho (by my read) that regex shouldn't match any tags inside the ..: that's what the ([^<]*) says. as such, it may be that i'm out to lunch on this, and an adjustment similar to mine above is actually in order. ;)
further testing/testers appreciated: can someone confirm that simply adding the tag to Allowed HTML tags resolves this issue?
#3
also meant to mention that i'm interested in volunteering to co-maintain this module, if that's still useful :)
#4
Hi spiderman,
I'd love to have you on board, check your email!
Just tested your suggestion about adding the tag to the list of allowed HTML tags. It didn't work for me. This is also the same for Full HTML (which automatically accepts all HTML tags) so it's not just a problem with Filtered HTML.
Here's what I used:
<go href="http://www.google.com/search?q=hello">hello world</go><go href="http://www.google.co.uk"> test <img src="http://www.google.com/intl/en_ALL/images/logo.gif" /> test </go>
The first link will work fine, but the 2nd link will just show an image with no link. Hope this helps.
#5
Spiderman has been added as a co-maintainer for this module.
-K
#6
ok following this up some more, it seems my initial hunch was correct- the regex that was looking for tags was excluding/stripping any with other html inside them. i've attached a proper patch to the module which fixes the problem for me. steps to recreate:
0. setup test/sandbox drupal 5.3 install
1. drop gotwo.module into sites/all/modules
2. apply this patch
3. enable gotwo module on admin/build/modules page
4. enable gotwo filter on admin/settings/filters/1 page (filtered html config)
5. rearrange filtered html weights so that the go filter happens first (or at least before html and url filters)
6. create a new page, with a link like the following somewhere in the text:
<go href="http://drupal.org">test <img src="http://drupal.org/themes/bluebeach/logos/drupal.org.png"> test</go>preview or save the page, and you should see the drupal logo with the word 'test' on either side of it, all as a link. also test removing the 'test' words, so the link becomes just the logo.
i'm quite sure this patch works to include img and other tags allowed by the html filter, so the only other issues i'm worried about are edge cases we haven't considered. anyone know the reasoning why this regex was so restrictive in the first place?
#7
Just tried this patch, works as advertised. I also tried various tests and it works as advertised.
RTBC :)
#8
i've just committed the patch. the updated version of gotwo.module is here.
thanks for the review, BioALIEN :)
#9
Automatically closed -- issue fixed for two weeks with no activity.