404 Error

[kr0n@…]

I know I can be a little tedious, sorry for that ;)

I realized that Plogger seems to doesn't have a 404 error. Instead of that, if you mis-typed an URL or simply goes to some inexistant album, Plogger redirects you without warning it to the parent correct address.

E.g If you go to

/plogger/true_collection_name/fake_album_name 

Plogger will redirect you to

/plogger/true_collection_name/ 

but the address appearing in your browser will still be the first one!!

I don't find this good for the user experience for diverse reasons, mainly usability and user confusion about this. IMHO the proper action to do here would be show an user-friendly 404 error, maybe with some useful links to redirect the user. But never show a web that wasn't requested and without noticing (even in the browser address bar!)

Well, that's the explanation. My implementation follows now ;) :

1) In the resolve_path() function I modify the following:

             // no such collection, return
		 if (empty($collection)) {
		 	header( "Status: 404 Not Found" );
			header($_SERVER['SERVER_PROTOCOL']." 404 Not Found");			
			return -1;
		 }

This piece of code is repeated in each if, for collection, album and image level.

2) As you may notice, in case of 404 error the resolve_path() function returns -1, instead of an array as expected. So the next point is to treat this scenario. In the gallery.php I added this:

[...]
// the followling line calculates the path in the album and excludes any subdirectories if 
	// Plogger is installed in one
	$path = join("/",array_diff(explode("/",$_SERVER["REQUEST_URI"]),explode("/",$_SERVER["PHP_SELF"])));
	$resolved_path = resolve_path($path);
	
	/*ADDED: No collection, album or image found. Bad url, redirect to 404*/
	if ($resolved_path==-1) {		
		$_GET["level"] = "BADURL";	
	} else if (is_array($resolved_path)) {
		$_GET["level"] = $resolved_path["level"];
		$_GET["id"] = $resolved_path["id"];
[...]

and in the the_gallery() function:

[...]
      $level = isset($_GET["level"]) ? $_GET["level"] : '';
	
	/*ADDED: No collection, album or image found. Bad url, redirect to 404*/	
	if ($level=="BADURL") {		
		putenv("REDIRECT_STATUS=404");
		/* ... treat properly the 404 error. 
            In mine, I used the REDIRECT_STATUS enviroment variable, 
            that is the reason of the previous line. 
            Yours may differ, of course */
	}	
	
	$allowed_levels = array('collection','album','picture','slideshow','search');
      [...]

That's all. I'm still testing it, so probably some retouch may be needed ;)

[mike]

The solution is already here in the comments, it just needs to be integrated with the codebase.

[kimparsell]

I'll look into integrating the solution listed with the current codebase.

[kimparsell]

This issue has been resolved in changeset 568 by a joint effort between sidtheduck and myself. Proper 404 errors are now generated (including headers) when using either default or cruft-free URLs. All themes in the download package now include a 404.php.

[sidtheduck]

Talking with Kim, I have edited the 404 errors a bit. With the patch for r569 attached to this ticket, 404 errors are only generated if that collection, album, or picture does not exist at all. If the album or collection exists, but does not have any images in it, the user receives a "Does not have any images" message instead of a 404 error message.

Would this be a better way to handle 404 error messages, or is the old way better (throwing a 404 error if the name does not exist or if it exists and no images are found within it)? The new way adds another SQL query during plogger_init to verify that the id of that object exists within the database. I'm looking for feedback.

Thanks.

[ryanduff]

Why not just display "Collection is empty" or "Album is empty"? It exists so a 404 is not exactly desired. There should be some feedback though that lets the user know that its empty.

[sidtheduck]

Replying to ryanduff:

Why not just display "Collection is empty" or "Album is empty"? It exists so a 404 is not exactly desired. There should be some feedback though that lets the user know that its empty.

This is what the new patch does, however it does add another sql query to the database to determine if the album or collection exists but does not have any images in it. Currently the plogger_init functions just check for collections or albums that have images in them (whether existing or not) and either throws a 404 error (for everything in the older patched code) or a "Collection or Album is empty" (with the new patch if the collection or album exists but is just empty of pictures).