Page 2 of 3

Posted: Thu Mar 13, 2008 11:50 am
by adamcharnock
Hi Ormus!

Its great to hear someone is getting some use out of this addition... or at least trying to!) :)

It sounds to me as though you are using the patch I initially posted. Garvin's later modification should have fixed the problem you are seeing with the body copy not being transformed. I would check out the version in CVS:

http://php-blog.cvs.sourceforge.net/php ... ectorplus/

In terms of using the image URI to identify the image, I agree that it would be more versatile - and that the current implementation will have problems with folders (thanks for pointing this out!).

However, the serendipity_admin_image_selector.php file only seems to take image ID as a parameter. So after having another look at the images table, I think there are two options:

1 - Match against the path column as well as name and ext
2 - Add the ability for serendipity_admin_image_selector.php to take an image ID or a URL.

Does anyone have any preferences here? Also, are there any existing functions to do this? (Not that I have a problem with doing it, I just don't want to start duplicating code).

Posted: Thu Mar 13, 2008 12:49 pm
by ormus7577
Ah, the id thing explains why serendipity_admin_image_selector.php hasn't changed. My vote is for allow both url and id as param :D

I've downloaded the cvs version, the problem is still there. The image id is 0 but it should be 6. When using the name and extension and sending the sql directly it however returns 6. Plus the fixed url (0->6) doesn't output an resized image but an image info page :-(

Posted: Thu Mar 13, 2008 1:16 pm
by adamcharnock
Humm, can you tell me what database you are using? I was only able to test on MySQL 5, so this could be a compatibility issue.

I have also noticed a change that has slipped into CVS and should not be there. The URL is generated on line 931 of serendipity_event_imageselectorplus.php, and it should read as follows:

Code: Select all

$newUrl = rtrim($serendipity['baseURL'], '/') . '/serendipity_admin_image_selector.php?serendipity[image]=%d&serendipity[disposition]=inline&serendipity[step]=showItem';
Note that "&serendipity[show]=resized" has been removed. This param required a change I made previously before Garvin pointed out the change was not necessary (and hence not included in CVS).

I would also recommend doing a var_dump() of both $sql and $row around line 925 and check that the values are expected. If you could post the values here it would be very useful.

I like the sound of changing serendipity_admin_image_selector.php to take URLs as well as IDs. I think it would be best to write a generic getImageIdFromUrl() method in the functions_images.inc.php file whatever happens.

Garvin/Judebert: Do you have any thoughts on this?

Thanks,

Adam

Posted: Thu Mar 13, 2008 3:02 pm
by ormus7577
Got it!

My fault: you have to activate dynamic resize in BOTH the plugin config AND the global s9y config. I wasn't aware of the global setting.

Your fault :wink: : I've var_dumped $m in the uploads branch preg_match:

Code: Select all

array(3) { [0]=>  string(31) "/serendipity/uploads/test/3.jpg" [1]=>  string(6) "test/3" [2]=>  string(3) "jpg" }
I guess you notice the problem, the path needs to be removed from the name...

Posted: Thu Mar 13, 2008 3:10 pm
by adamcharnock
Hi Ormus,

Thank you for tracking this down for me! My blog is still fairly new so I have yet to orgnaise my images into folders :p

I will also make it clearer that global image resizing needs to be enabled from the get-go.

I will go write that getImageIdByUrl() method now :)

Posted: Thu Mar 13, 2008 3:17 pm
by garvinhicking
Hi!

I am actually in favor of modifying the plugin to match directory+filename+ext. Letting the image-selector take a URL as parameter both can make the query slower, might have more SQL exploitation issues and is generally really long, which I'd like to avoid.

Plus, it can be done without patching s9y, and so the plugin can work independently from s9y 1.4 (because this patch wouldn't make it into the 1.3 release).

Regards,
Garvin

Posted: Thu Mar 13, 2008 3:58 pm
by ormus7577
Hi,

I can live with the id thing (if the id is selected by using name, path and extension).

I think I misphrased a former question:

I would like the plugin to parse html outside the actual entry. Example: The index.tpl generates some html containing an image (which can be found in the images database, like the frontpage images of the mimbo template or some category icon...). I would like to change that image url as well to be resized. Is there any way to do this? (And sorry if it has been answered and I just didn't understand it...)

Posted: Thu Mar 13, 2008 4:03 pm
by adamcharnock
Ok, that sounds fine to me. Here is the patch aganst CVS:

http://porteightyeight.com/serendipity_ ... 0313.patch

Or if you prefer the whole lot again:

http://porteightyeight.com/serendipity_ ... 313.tar.gz

I have noticed that my part of the code gets run once when a post is saved and again when it is displayed. The second time round is redundant because the URLs have already been transformed. I cannot see an obvious reason why this is happening - so if any could fill me in I would be very grateful.

I have also still to add a warning about globally enabling auto image resizing, I just wanted to get a patch to Ormus so he (she?) can give it a go.

There may also be some merit in moving the getImageIdByUrl method into functions_images.inc.php - but I will leave that one up to Garvin.

Hope it works ok :)

Posted: Thu Mar 13, 2008 4:12 pm
by adamcharnock
Hi Ormus,

I see what you mean, and I think it is a good idea. How would you feel about this being a smarty plugin? I am convious that doing regexs on large chunks of HTML can be slow, and a smarty call could make this faster: Eg:

{resizeImage src="blah.gif" alt="example" border="0" class="myclass" ...}

Each of the params would basically be an attribute in the img tag, with the exception of the src tag that would have the obvious pre-processing applied to it.

Does anyone have any other/more thoughts on implementing this?


Adam

Posted: Thu Mar 13, 2008 5:32 pm
by ormus7577
Hi Adam,

Thanx for the patch. Your ltrim parameters are swapped, but once they are put into correct order everything is working now! Beautiful! (side note: there's a typo in the plugins config, it's should be height for the dynamic resize switch label)

About the Smarty thing, I didn't know that this would be possible (as I never really used smarty). But sure, if that's possible that is great and a much better way than doing markup on the final html... Do you know your way around smarty?

Posted: Fri Mar 14, 2008 9:52 am
by garvinhicking
Hi!

I committed your patch, with two modifications: I bumped the verison number, and replaced some occuring tabs with 4 spaces.

Ormus, what about the ltrim parameters? They seem to be in the right order to me!?

Regards,
Garvin

Posted: Fri Mar 14, 2008 12:05 pm
by ormus7577
garvinhicking wrote:Ormus, what about the ltrim parameters? They seem to be in the right order to me!?
He updated the .gz file between you and me downloading ;)

Adam, I thought about the smarty thing you mentioned. Wouldn't that also mean, that smarty needs to know the image id? My problem is, that I need to know the image id to apply the resize. But I can't put the image id in a extended entry property field (without doing this manually)...

Posted: Mon Mar 17, 2008 9:18 pm
by judebert
It's starting to sound like we really need to update Smarty's image functions. We're also talking about getting a size for images so the caption block can be floated properly.

So, we're up to: makeImageContentBlock and getImageId?

Posted: Mon Mar 17, 2008 11:20 pm
by adamcharnock
Hey Guys,

Sorry for being a little quiet lately, I am currently off holidaying at the moment so I am stuck on dial-up on my laptop :)

If I can be of help with this please let me know. I believe there is a function for getImageId in the above patch, so you are welcome to use that as a basis if it is useful.

Posted: Sun Mar 23, 2008 10:45 am
by ormus7577
Here's another patch for that plugin. I suggest extending the sql command on line 779 to check for the extension as well. Because if the user stores other files (pdf, flv etc.) in the media db then this will cause division by zero exceptions, as no dimensions are set.

Code: Select all

and UPPER(i.extension) IN ('JPG','PNG','GIF')