Closed GoogleCodeExporter closed 8 years ago
The main issue is the database queries which increases currently if we use the
current rewrite rules and fetch the gallery/album name for better permalinks.
My idea
is to rewrite the ngg-db class and use a object cache so that the database will
not
be again and again hit for the same request.
If I have rewritten this part, then nggRewrite can simply fetch the slug of a
gallery
or album
Original comment by alex.cologne
on 29 Aug 2009 at 7:56
Are you about to rewrite ngg-db, or is it just an idea?
Original comment by prollius@gmail.com
on 31 Aug 2009 at 12:18
Yes, I started to use wp_object_cache, but this need more time. The rewrite
rules are
(for me) not that easy to implement. Many,many Regex and special behaviors in
the WP
internal rewrite system...
Original comment by alex.cologne
on 1 Sep 2009 at 6:59
any updates on the permalinks?
Original comment by sybold
on 13 Jan 2010 at 7:06
Yes please post an update on this because the current link suck bigtime :)
Original comment by i...@abcleaning.nl
on 20 Jan 2010 at 10:24
Agreed, is there any updated news to this feature?
Original comment by suh...@gmail.com
on 6 Mar 2010 at 7:14
I am already in process of creating the plugin for this feature, but I am
willing to
implement it direct into the plugin. Is it possible to join the team (I know it
is one
man ride)?
Thank you.
Btw I also have ideas for improvements on other areas of plugin, and also
willing to
implement them.
Original comment by mario.ko...@gmail.com
on 29 Apr 2010 at 9:20
Yes you can contribute, contact me by mail or attach patch files
Original comment by alex.cologne
on 30 Apr 2010 at 6:23
this would facilitate adding commenting functionality on individual photos,
which
would be amazing. Mario, could you notify us as soon as you have something?
even if
its in alpha, as this would be enormously helpful to me.
Original comment by igoo...@googlemail.com
on 17 May 2010 at 11:22
This evolution in the permalinks would be fantastic! I'll be sure to keep an
eye out for it. Thank you for your ongoing development on this plugin, it's
greatly appreciated.
Original comment by iverson....@gmail.com
on 10 Jun 2010 at 9:14
[deleted comment]
Thanks for all your hard work on this! I'm ready for the permalinks as I have a
client that really needs it to run his site properly. Please notify me as soon
as you get it up and going. Much appreciated guys!
Original comment by 68k...@gmail.com
on 15 Jun 2010 at 5:40
Thank you for your work ! I wait for this new permalink
Original comment by easyso...@gmail.com
on 18 Jun 2010 at 10:56
Just to let people know that I have also developed full working code for nice
SEO-friendly permalinks for NextGEN.
I'm planning to have them rolled up into a distributable "mod" style package in
the next few days - the code at the moment consists of hacks made to a raw
install of NextGEN, so no configuration or options.
I'll update when I have patch files to contribute, but wanted to keep people
here informed :)
Original comment by craig.watson@youdevise.com
on 6 Jul 2010 at 1:16
Sounds good. Contact me if you have any questions about the code
Original comment by alex.cologne
on 6 Jul 2010 at 7:45
I've attached a tar.gz of the complete NextGEN archive.
Changed files are:
nggfunctions.php
lib/ngg-db.php
lib/rewrite.php
There are several code changes, so I won't list all changes here. After
replacing the files, log into your wp-admin console and go to your NextGEN
options. Click on the "Permalinks" tab, and the options should be
self-explanatory.
Note to anyone upgrading to this patch, obviously there is a risk in using this
as it I haven't tested on anything other than my own website. Remember to
backup your current nextgen-gallery plugin folder, as well as any customised
CSS files you have.
Alex - I can provide .patch or diffs to either ngg-1.5.5 or the SVN development
trunk if you need them.
Demo gallery is here: http://www.cwatson.org/gallery
WP version: 3.0
NGG version: 1.5.5-cw
Original comment by craig.watson@youdevise.com
on 6 Jul 2010 at 6:42
Hi, thank you for your plugin. i've tested it on my page and unfortunately it
doesn't work in my case :(
I think the problem is that i'm using the image browser and not the js viewer.
If you want to have a look at my setup you can see it at http://www.gamue.de
Original comment by johannes...@gmail.com
on 6 Jul 2010 at 8:12
Johannes - Try editing line 265 on lib/rewrite.php and change "page" to "image"
- I believe this is an error in the original rewrite.php because my rules are
copy-pasted from the originals.
Hope that works :)
Original comment by craig.watson@youdevise.com
on 6 Jul 2010 at 8:23
Also, another minor glitch, I managed to leave the development init hook for
rewrites enabled (oops!), so comment out line 32 (if you're unfamiliar with
code, add // before the command "add_action(......".
I've attached a replacement set of files :)
Original comment by craig.watson@youdevise.com
on 6 Jul 2010 at 8:29
thanks, that change make it work!
Original comment by johannes...@gmail.com
on 6 Jul 2010 at 8:32
I had some troubles to review your changes as you have a Mac/Windows Double
spacing issue included (EOL Conversion), so I hope know I get all your changes
into one patch file as attached.
I can see you changed four files :
nggfunctions.php
lib/ngg-db.php
lib/rewrite.php
admin/settings.php
Correct ?
Original comment by alex.cologne
on 7 Jul 2010 at 8:34
Hi Alex,
Yep, that patch file looks right. Apologies for the characterset problems, I'll
look into them now and replace the attached archives as soon as I have fixed it
:)
Original comment by craig.watson@youdevise.com
on 7 Jul 2010 at 8:47
From my first review it look really good, I need to check all combinations
(over the time I have included a terrible amount of combinations between
album/subalbum/gallery/image etc...) and as well I must look if this not open
any SQL injection (the main reasons why I handle till now integers instead
strings).
Your solution has one limitation (from that what I can see) it just handle one
gallery landing page, some user include galleries into their post and there it
will not work.
Original comment by alex.cologne
on 7 Jul 2010 at 8:54
Thanks Alex,
I am aware of the problem with posts vs pages, but I think this is the main way
that people expect a gallery to function.
I have made a test blog post on my own website here:
http://www.cwatson.org/2010/07/07/test-gallery-post/
It seems like a minor problem, so hopefully I'll look into it today and have a
fix for you :)
Original comment by craig.watson@youdevise.com
on 7 Jul 2010 at 9:17
If I click on a gallery at your test page i'll get a 404, i think that's the
error you're talking about? in my test setup yesterday, i have a page with an
album inside which worked. one think i was wondering about is that the album
number isn't changed to a name. It's the same in your example
"http://www.cwatson.org/gallery/post/test-gallery-post/7/saxon_-_london_astoria_
-_april_2006/" (after test-gallery-post is the number). do you think it's
possible to change this number into the galleryname or isn't it?
another thing which i saw is that the slug is required, could it be optional or
not?
Original comment by johannes...@gmail.com
on 7 Jul 2010 at 9:24
I think this can be solved, I'm out now and will deeper test this tomorrow.
Thanks for all your input
Original comment by alex.cologne
on 7 Jul 2010 at 9:28
Seems to be fixed now, I originally didn't include the post links in the SEO
array in lib/rewrite.php, so they weren't being parsed - and I also missed one
line out of the nggfunctions.php file when dealing with posts.
I've attached both changed files :)
Original comment by craig.watson@youdevise.com
on 7 Jul 2010 at 10:01
Looks like there's another problem with some characters, if you want to see the
"Iron Maiden - Earl's Court - December 2006" Gallery it won't work
Original comment by johannes...@gmail.com
on 7 Jul 2010 at 10:05
Should also mention that the permalink structure needs to be manually updated
to take into account the new rules in rewrite.php
Original comment by craig.watson@youdevise.com
on 7 Jul 2010 at 10:05
[deleted comment]
Lines 348 and 349 respectively of nggfunctions.php become:
$gallery = str_replace("_","
",htmlspecialchars(stripslashes($gallery),ENT_QUOTES));
$album = str_replace("_"," ",htmlspecialchars(stripslashes($album),ENT_QUOTES));
Problem was just the single quote in the URL not being translated to ' (it's
XHTML ASCII code).
Alex - as far as I can tell it should be fairly OK for XSS and SQL injection,
as $wpdb->prepare will escape the SQL for you :)
Original comment by craig.watson@youdevise.com
on 7 Jul 2010 at 10:41
Hi maidenfan,
I've just tested the new version and get the following error if i use the last
nextgen-gallery.zip archive you've uploaded and get the following error:
Warning: Cannot modify header information - headers already sent by (output
started at [..path..]\wp-content\plugins\nextgen-gallery\lib\core.php:931) in
[..path..]\wp-includes\pluggable.php on line 890
so i've used the archive from yesterday and just replaced the nggfunctions.php
and rewrite.php files.
As i mentioned before i'm using the image browser on my page, now i'm getting a
404 error again, the path is e.g.
http://.../photos/testalbum/testgallery/image/1. For a test i've reverted the
page-to-image change, and then it works. So if I use
http://.../photos/testalbum/testgallery/page/1/ the image will be displayed.
It's a little bit confusing...
Original comment by johannes...@gmail.com
on 7 Jul 2010 at 8:48
I've also found a problem with german umlauts like ä,ö and ü. I think some
other special characters (like french ones) could also be effect an error.
A gallery called "täst" is transformed into the follwing url
http://localhost/wordpress/photos/testalbum/t%EF%BF%BDst/ after clicking it i
get a "gallery not found"-message. The correct UTF-8 code for ä would be %C3%A4
Original comment by johannes...@gmail.com
on 7 Jul 2010 at 8:58
Hi Johannes,
I've not got any errors from core.php, I think there's a bit of confusion about
the "current" version of files, so I have included a complete set here, and
will delete the rest.
I've posted files, complete archives and manual line-edits in this discussion
thread so it may be confusing!
After replacing the files, refresh your permalink structure and thinks should
be all working (I hope!).
I've also tested the code with special characters, and have a demo gallery here:
http://www.cwatson.org/gallery/we_will_%22r%C3%B6ck%27_you/s%C3%B6me_g%C3%A3ller
y_with_l%C3%B6ts_%C3%B6f_specialchars_&_o%27er_%22stuffs%22/
Hopefully that URL is enough of a proof of concept ;)
Alex,
Not sure what the timescale is for the next NGG release, but if you could
re-compile the patch from the changed fileset. Any other bugfixes I do here
will detail what line/file to replace and the new lines(s) - hopefully this
will make things easier to follow :)
Thanks,
Craig
Original comment by craig.watson@youdevise.com
on 8 Jul 2010 at 8:30
Attachments:
Attached the new patch file from the latest version. (it's really hot here, so
have didn't work that much... waiting for a cool down :-)
@Craig
There is no current time scale, I'm working on some other topics (see
changelog.txt) and when I feel a version is stable, I will release it. As I can
see from your concept I have a design failure in my plugin : Album and gallery
didn't have a unique "slug" which we need to build up a correct SEO friendly
URL. As we can see from the umlauts issue the gallery name needs to "sanitize"
from special chars in the same way as WordPress sanitize a blog title for the
permalinks.
Nevertheless your idea is the best we can have for now, so I will test it and
let you know if I will add it to core. Thanks for all the work.
Original comment by alex.cologne
on 9 Jul 2010 at 2:13
Attachments:
Hi Craig,
thanks for the new file set now the header from core.php disappeared. I still
have the umlaut problem, but maybe there's a misconfiguration i've done, i will
have a look at it.
But if i click on an image (using image browser) i get the following error,
could you please test it? if it works i'll delete my local environment and
create a new one.
URL is: http://localhost/wordpress/photos/testalbum/testgallery/image/2
Error is:
Warning: Cannot modify header information - headers already sent by (output
started at ...\wp-content\plugins\nextgen-gallery\lib\ngg-db.php:1575) in
...\wp-includes\pluggable.php on line 890
Thanks for your amazing work, can't wait to use it online.
Original comment by johannes...@gmail.com
on 10 Jul 2010 at 10:29
Alex and Johannes:
I'm pretty busy for the next week or so with freelance web design projects, so
my time is fairly limited.
I think the best thing to do is go back and re-develop the SEO fix properly,
including a "sanitised" gallery/album title database field to make things a lot
easier.
If Alex can implement this before I have the chance to look at it, that would
be brilliant, but if not then I'll look at it when I can (hopefully week
beginning 19th July). I'll hopefully do a "complete" solution to save Alex the
trouble :)
We'll get there in the end :)
Craig
Original comment by craig.watson@youdevise.com
on 12 Jul 2010 at 6:55
There is no rush... it's a hot summer here and I'm working on the plugin not
very often, so take your time and enjoy the summer :-)
Original comment by alex.cologne
on 13 Jul 2010 at 6:49
Alex,
Sorry it's taken a while, a lot of things have been happening recently!!
I should be able to take a look at NextGEN again this week. What I plan to do
is do a fresh set of patches against the current trunk versions of NextGEN, and
include database schema changes.
I'll hopefully make some changes to the update mechanism to take into account
the changes.
I think it's safer to just treat this as a NextGEN code submission rather than
a manual patch. Hopefully then it's just a case of testing the changes and then
releasing them as part of NextGEN :)
Cheers,
Craig
Original comment by craig.watson@youdevise.com
on 4 Sep 2010 at 3:29
take your time, I will release in two weeks V1.6.0, then I will start the next
cycle and we can look into this. Thanks for all your help !
Original comment by alex.cologne
on 4 Sep 2010 at 4:59
Alex,
Is there chance in getting this into 1.6.0? I'm pretty much done with the
changes now - they took me a lot less time than I planned!
I'll post up a patch file against the current trunk when I've verified it all
works. I've done some minor tweaks to enable SEO permalinks of individual
images - something people have been asking for I think.
One minor thing I need to correct is getting the information into the XHTML
<title> tag, but that shouldn't take too long :)
If it's possible to integrate the code and release it as 1.6.0b4, that would be
cool :)
Cheers,
Craig
Original comment by craig.watson@youdevise.com
on 5 Sep 2010 at 8:17
I don't prefer this : I'm a one-man-show and need to test all features and
functions as good as possible, V1.6.0 test cycle is finished and I wait now one
week until I release this version, right after the release we can start the
next version and include your patches, so there is enough time to test it...
Original comment by alex.cologne
on 5 Sep 2010 at 8:24
No problem, I appreciate that testing is a very big task - I've tried to test
my code as much as possible, and it does seem to work as intended with no
side-effects.
I'll keep working anyway - I'm away from 17th September for 2 weeks (holiday!)
so ideally things will be tested enough to integrate then :)
Original comment by craig.watson@youdevise.com
on 5 Sep 2010 at 8:37
Hi Alex,
I've just finished the work on the Permalinks - a working example is online
here: http://www.cwatson.org
I've attached both a zip archive of NextGEN (1.6.0b3 plus my changes) and a
patch file taken from the current SVN trunk.
I'm not sure about whether it's safe to distribute the patched files, given the
nature of the database updates, but I've changed the database version to
1.5.9-cw (pretty much 1.6.0 but with my own modifications) so that it doesn't
interfere with any 1.6.0 updates if it is applied. If this isn't suitable, let
me know and I'll change it.
There does seem to be a bug within lib/ngg-db.php within the find_image
function. When embedding an ImageBrowser into a post, the WP cache throws up
some errors about incorrect types (wish I had an example now!). Strangely, this
doesn't happen when using ImageBrowsers within a page, and it solvable by
commenting out the caching lines within find_image.
I appreciate if you're busy with 1.6.0 at the moment, though :)
Craig
Original comment by craig.watson@youdevise.com
on 6 Sep 2010 at 1:20
Take a quick view into your patch and it looks fine, I would recommend some
minor tweaks for the db schema :
nggallery contain already "name" and is normally not used (just as shortcode
tag) so this should be used as sanitize slug, same can be done for nggalbum ->
"name" can be sanitize and must be unique, for nggpicture I suggest to name the
field "image_name" instead "alttext_sane", would be more descriptive
In two weeks I will start on this, wish you some nice holidays. Thanks for your
contribution !
Original comment by alex.cologne
on 6 Sep 2010 at 7:06
One issue about sanitize_title($alttext), how do you ensure that it's unique ?
A user can use as alttext several times the same descriptive text (ie. 'Summer
2010')...
Original comment by alex.cologne
on 6 Sep 2010 at 7:12
I'm not sure about using the nggallery "name" as the sanitized version, the
title could be more descriptive and user-customisable so maybe it's best to
keep the title as the slug? This way the user can change it and not affect any
existing shortcodes.
The database schema changes otherwise look good, I'd suggest prepending the PID
to the alttext, so "Summer 2010" becomes "123-summer-2010". This would also
need some tweaking on the rewrite side to just send the "123" part to NextGEN
to find.
Just out of curiosity, is there ETA on v1.7?
Original comment by craig.watson@youdevise.com
on 6 Sep 2010 at 12:00
V1.7.0 will released shortly before WordPress 3.1 as some functions now
deprecated.
I will look in 2-3 weeks for the unique slug process, WordPress must have a
inbuilt function already as the slug/permalink for a post is always unique.
Original comment by alex.cologne
on 6 Sep 2010 at 7:49
The post/page permalink uses the sanitize_title() function, which is the same
function I've used when sanitizing the NextGEN data.
The reason I chose the gallery's "title" attribute to be the permalinked one is
that it's user-configurable, and it matches the displayed data - an SEO point
really - rather than the shortcode.
When the user changes the gallery's title, the permalink should change, in the
same way as a post/page. If the title already exists, WP appends a number to
the end of it, so perhaps this is a solution to the uniqueness problem?
Another solution could be to make the title/name/whatever a key (I know that
MySQL can handle multiple primary keys, not sure about other DBMS')?
Original comment by craig.watson@youdevise.com
on 6 Sep 2010 at 8:11
Correct, sanitize_title() is used in combination with a counter, so the
function need to be extended in the same way as you have written... think a db
lookup before saving is needed in this case
Original comment by alex.cologne
on 6 Sep 2010 at 8:17
Original issue reported on code.google.com by
prollius@gmail.com
on 26 Aug 2009 at 10:10