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>

CommentFileSizeAuthor
#6 gotwo.101605.patch657 bytesspiderman

Comments

BioALIEN’s picture

Version: 4.7.x-1.x-dev » 5.x-1.3
Priority: Normal » Critical

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?

spiderman’s picture

Assigned: Unassigned » spiderman

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 Only local images are allowed. tags were being stripped by the default Filtered HTML settings. once i added Only local images are allowed. to the admin/settings/filters/1/configure page (Allowed HTML tags), my test node with a Only local images are allowed. 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:

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 Only local images are allowed. 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 Only local images are allowed. tag to Allowed HTML tags resolves this issue?

spiderman’s picture

also meant to mention that i'm interested in volunteering to co-maintain this module, if that's still useful :)

BioALIEN’s picture

Status: Active » Needs work

Hi spiderman,

I'd love to have you on board, check your email!

Just tested your suggestion about adding the Only local images are allowed. 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.

Zen’s picture

Spiderman has been added as a co-maintainer for this module.

-K

spiderman’s picture

Status: Needs work » Needs review
StatusFileSize
new657 bytes

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?

BioALIEN’s picture

Status: Needs review » Reviewed & tested by the community

Just tried this patch, works as advertised. I also tried various tests and it works as advertised.

RTBC :)

spiderman’s picture

Status: Reviewed & tested by the community » Fixed

i've just committed the patch. the updated version of gotwo.module is here.

thanks for the review, BioALIEN :)

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.