Markup: Gallery Image bug

[scottwalsh]

Have found a bug in the plugin for Gallery Markup. (serendipity_event_galleryimage.php)

I’m using gallery2 and have found the plug in does not work correctly if you have albums within albums (i.e. subalbums).

Have traced this back to two issues.

One

When the plugin parses the markup, it uses a regex to generate a url. If the markup is:

Code: Select all

[GImage]album/subablum/image[/GImage]

the url for the image is generated as:

Code: Select all

http://site//plugin/g2wrapper?album=ablum&image= subablum/image&ext=jpg

instead of

Code: Select all

http://site//plugin/g2wrapper?album=ablum/subablum &image=image&ext=jpg

The regex is on line 421:

Code: Select all

$output = preg_replace("'\[GImage\s*([^\]]*)]([^\/]*)/([^\.<]*)\.*([^<]*)\[/GImage]'$preg_flags", "\$this->gimage_thumb('\\2', '\\3', '\\4', trim('\\1'))", $text);

I’m a bit out of practice for correcting the regex, so would be great if some could have a look at what it should be (otherwise I’ll look when I have more time).


Two

Even if manually correct the image url, the image does not display due to the way the url is parsed by the event_hook function.
When it parses it, album=ablum/subablum has the / removed, so can not find the image.

This can be corrected by removing line 351:

Code: Select all

$album = str_replace("/", "", $album);

Thanks

Scott
http://zone3.net.nz/[/quote]

[garvinhicking]

Hi!

Actually, this is not really a bug but more a missing feature of the plugin -- it simply does not evaluate sub-albums.

So we can't just "fix" the regular expression. It would need to be coded quite differently, so that it can deduce whether a path/subalbum or an image is referenced...

Sadly I myself don'T use gallery and haven'T written the plugin, so I'm not really sure how support for subalbums could easily be added...

Regards,
Garvin

[scottwalsh]

Hi Gavin,

From working through how the code works, I’m pretty sure resolving the regex will solve the issues am having with sub-albums.

The second issue takes a url and generates an image. I was able to fix this to work with sub-albums with a minor change.

The first issue around the regex is used to generate the url. So if the regex is changed to correctly generate the url, am pretty sure it’ll work.

I’m happy to test it and work on getting this to support sub-albums, but I do some advise on regexs.

If anyone can help with the below, I should be able to test and provide a patch.

If I have a regex of

Code: Select all

([^/]*)/(.*)

and pass in the string aa/bb/cc/dd, everything up to the 1st / ends up in \1 and the rest in \2.
I’d like to get a regex that takes a string and puts everything up the last / in \1 and after the last / into \2…

[garvinhicking]

Hi Scott!

Ah, now I understand. I'm sorry, I didn't fully recognize that just the first portion only returning a path should fix the issue.

Maybe it already works when using an ungreedy preg pattern.

Try to use this code before the preg_replace you quoted:

Code: Select all

$preg_flags = ($case_sensitive) ? 'Ue' : 'Uei';

(note the added 'U' to the flags)

Does that help?

Regards,
Garvin

[scottwalsh]

Hi Gavin,

Tried it, still doesn't work though.

Don't think it is a greedyness issue as the regex uses a negated / to avoid the greedyness...

I'll have another look at the regex tomorrow

[garvinhicking]

Hi!

Would

([^/]+)/(.*)

maybe work in that case? To require a first-portion pattern match to stuff the rest into a different match pattern?

Regards,
Garvin

[scottwalsh]

Hi Gavin,

That didn't get it either. Didn't quite work out the regex, but changed the way the parsing is done a bit and solved getting the plugin to support subalbums (also now supports images in the base album).

Spent a bit of time testing it and seems to work fine agaist gallery2.

I've created a patch file agaist v1.4 that incl this feature and put it http://zone3.net.nz/galleryimage.patch.txt

Happy for it to get rolled into the plugin in case anyone else is looking for this feature,,,

[garvinhicking]

Hi!

Great, many thanks for that. I couldn't test it, but the code looks sane to me, so I committed your patch.

Regards,
Garvin