5.4.0 Bug With Pretty URLs And Added Paths?

Permalink
Before I submit this as a bug, I would like someone else to confirm that it is in fact not the intended behavior. Here's what I observe...

Pretty URLs are working just fine EXCEPT when referencing a page via an added path (i.e. an "additional URL" added through the page's properties). So for instance, I have a page called "staff" under an "about" page. I have added an additional URL of "/staff" so that it can be more easily remembered and more readily accessed (such as when publicizing the URL in a radio spot or something - e.g. "For more info, just go to mysite.com slash staff." (The less folks have to remember, the better.) Anyway, here's the issue... If I go towww.www.mystie.com/about/staff,... then, as expected, index.php does not appear in the URL. Good. However, if I go tohttp://www.mysite.com/staff, then it redirects towww.www.mysite.com/index.php/about/staff... - i.e. the canonical URL but with index.php. Is this the intended behavior, or is it a bug? In other words, will pretty URLs simply not work with non-canonical URLs?

-Steve

Shotster
 
glockops replied on at Permalink Reply
glockops
I do not seem to have this problem with my site running 5.4 for existing or newly created additional page URLs.

Did you create the additional page paths after upgrading?

On a completely different topic, is your dashboard sitemap working properly - mine broke ;(
Shotster replied on at Permalink Reply
Shotster
Thanks for the reply, glockops.

> I do not seem to have this problem with my site running 5.4 for existing or newly created additional page URLs.

Then I guess it's safe to assume that what I'm experiencing is NOT the intended behavior. So then, I have a question for you... When you reference a page with an additional URL, does it redirect to the canonical URL, or does it show the additional URL in the address bar?

> Did you create the additional page paths after upgrading?

Yeah, I'm pretty sure, but I'll add some more to double check.

> On a completely different topic, is your dashboard sitemap working properly - mine broke ;(

My sitemap seems to be working fine.

-Steve
Shotster replied on at Permalink Reply
Shotster
Ok, so here's what I found upon stepping through the code...

The following constant declaration appears in concrete/config/app.php...

if (!defined('URL_REWRITING_ALL')) { 
    define("URL_REWRITING_ALL", false);
}

Therefore, the following conditional statement in the url() method of concrete/libraries/view.hp will always evaluate to true...

if ((!URL_REWRITING_ALL) || !defined('URL_REWRITING_ALL')) {
    $dispatcher = '/' . DISPATCHER_FILENAME;
}

If I simply replace the above with "if (false)..." pretty URLs work as I would expect - that is, a non-canonical URL is redirected to the canonical URL but WITHOUT the index.php in it. So in short, it seems there's a bit of flawed logic here.

FWIW, the URL_REWRITING_ALL constant is defined as false in app.php and appears in only a couple of files - view.php and navigation.php. I won't pretend to understand its purpose since there's also a URL_REWRITING constant, but its use in navigation.php is as follows...

if (!defined('URL_REWRITING_ALL') || URL_REWRITING_ALL == false) {
    if ((!URL_REWRITING) || $ignoreUrlRewriting) {
        $dispatcher = '/' . DISPATCHER_FILENAME;
    }
}

...which seems rather odd since it's always defined and always false, which means the outer conditional will always evaluate to false.

It therefore seems the logic needs to be examined wherever the URL_REWRITING_ALL constant is used throughout the C5 codebase, which seems to be only a couple of places.

-Steve
andrew replied on at Permalink Reply
andrew
It's (somewhat) working as intended, although it's a little bizarre. The way URL rewriting originally worked, when you turned it on, immediately all URLs, whether they came from front-end navigation items or the url() function that you're referencing, removed index.php. This meant that the dashboard link linked to /dashboard, rewrites like the one you're mentioning would redirect without index.php in them, etc...

However, when people turned on pretty URLs, many of their environments wouldn't automatically add the .htaccess file, or there'd be some other hangup. With the old system, everything was essentially screwed at this point: there'd be no way to navigate back to the dashboard, and furthermore none of the forms would submit to the correct spots (because they, too, were using the url() function which, now that pretty URLs were active, was stripping out index.php)

So we settled on a compromise. Turning on pretty URLs removes index.php from all page lists, autonavs, etc... - basically anything that uses Loader::helper('navigation')->getLinkToCollection($page); The View::url() function itself, however, would retain index.php so that it would be possible for a user to turn OFF pretty URLs.

However, since we do know that some users (including ourselves) are technically savvy and want index.php gone from EVERYTHING, we created the URL_REWRITING_ALL constant. This will remove index.php the way it used to - from EVERY url() call, from the dashboard, from links to /login/, and, as in your example, from canonical urls when they are redirected. So now that you have pretty URLs working, just add

define('URL_REWRITING_ALL', true);


to your config/site.php and the behavior should be setup properly.
Shotster replied on at Permalink Reply
Shotster
Ahhh, Ok! Thanks, Andrew! I figured there was more to it than meets the eye, although I'm not sure where I should have looked to learn the bit about changing the constant. Perhaps I missed it in the docs or comments somewhere. Easy enough to do though...

-Steve
mose replied on at Permalink Reply
mose
Generally, if you are using pretty URLs, you should define URL_REWRITING_ALL to be true in config/site.php.
andrew replied on at Permalink Reply
andrew
However, I think the complaint is legitimate. Our approach to pretty URLs in this way was really to ensure that it was possible to undo them. Changing the canonical url behavior doesn't affect this and it makes sense to me.

In this way, try using this as a fix (which we will include)

Open concrete/startup/external_link.php

and change

header('Location: ' . BASE_URL . View::url($c->getCollectionPath()), true, 301);

to

header('Location: ' . Loader::helper('navigation')->getLinkToCollection($c, true), true, 301);
pgee replied on at Permalink Reply
Andrew/Anyone

Can you help me..?

I know this is a while ago for some of u here..but I'm going nuts trying to get my site back after a godaddy apache upgrade from 2.0 to 2.4 blew it away..

it relates exactly to the pretty urls not working?

site.php , .htaccess and library.php files updated dozens of different times and ways.. no joy...

I'm on shared hosting.

when I enable pretty urls - I get 'input file not found'

Phil