search

fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

Input::uri() causes Router and Uri::create malfunction #830

Closed antitoxic closed 12 years ago

antitoxic commented 12 years ago

Input::uri always strips out the extension in the uri. This makes a reverese Uri::create and Router to malfunction.

If you have a route defined as:

'test.html' => 'welcome/test'

The URI against which the route will be matched is test, not test.html.

Same logic applies to URI::create - it will use the test, not test.html as base to create a url.

antitoxic commented 12 years ago

This concerns 1.1/develop . Strangley the master seems to have a newer commit which is not applied in the develop branch: https://github.com/fuel/core/commit/4070ffccbde77ff7837fff8ddc49847ecb3a8ba7

WanWizard commented 12 years ago

You should not define routes with extensions. Routing happens on URI segments, the extension is irrelevant for a FuelPHP application. That it used to work was considered a bug, that has now been fixed.

As to that commit to master, that looks odd, we'll check that...

WanWizard commented 12 years ago

Looked at that commit.

It is not newer, it is a 6 month old commit. And two weeks after that commit, Dan rewrote the URI detection logic, in which the detect_uri() method was removed. That's why you don't find it in Input today.

antitoxic commented 12 years ago

@WanWizard that means the correct version is into the develop branch?

Does this then mean Uri::create has an error by not appending the detected extenison to the uri.

EDIT: Wait, how can the developer force a route with html? .htaccess ?

WanWizard commented 12 years ago

The extension is meaningless in FuelPHP, it is only used for REST services.

There is no way to route 'this/that.html' different from 'this/that.jsp'. The extension is irrelevant and ignored. As said, you should not route on extension. The old behaviour will not be restored, as it would break the REST functionality.

antitoxic commented 12 years ago

@WanWizard Thank you for the clarfification.

What would you recommend in situation where the SEO department requires certain links (articles ,news, certain pages) to have the html extension in the url?

julesjanssen commented 12 years ago

Aside from being irrelevant for SEO purposes, use the 'url_suffix' setting in config/config.php.

antitoxic commented 12 years ago

@julesjanssen

That will make all urls end with '.html'. I have to make only part of them.

I thought of manually calling Config::set('url_suffix','.html') in the before() controller method but that will apply to all of the generated URIs.

julesjanssen commented 12 years ago

You can also use \Uri::create('something/something.html') to add a suffix to just that URL.