Bug Report #1942

The Request class should transparently convert dashes to underscores

Added by Jonathan Geiger about 5 years ago. Updated about 4 years ago.

Status:ClosedStart date:08/12/2009
Priority:NormalDue date:
Assignee:Sam de Freyssinet% Done:

0%

Category:Core
Target version:v3.1.0
Resolution:wontfix Points:

Description

Currently, a route such as about-us/mission-statement results in an exception thrown because Kohana attempts to instantiate a controller named Controller_about-us. A similar thing happens with action names. Obviously, dashes are not valid in class or function names in PHP.

I've attached a patch to fix this.

It is important to note that this patch only converts dashes to underscores when instantiating the controller and calling the action. That is to say, its will not affect Request::instance()->controller or Request::instance()->action. Whether this is desired is up to the developers.

request.patch Magnifier (345 Bytes) Jonathan Geiger, 08/12/2009 09:47 PM


Related issues

Related to Kohana v2.x - Feature Request #2242: Change dashes to underscores when looking for routes and ... Closed 10/16/2009

Associated revisions

Revision a3020e20
Added by Woody Gilk about 5 years ago

Controllers and actions with hyphens should be converted to underscores, fixes #1942

Revision 3f6919be
Added by Woody Gilk about 5 years ago

Revert "Controllers and actions with hyphens should be converted to underscores, fixes #1942"

This reverts commit a3020e207e3b506e7d081336d028aeb3ee5bf1ca.

History

#1 Updated by Jonathan Geiger about 5 years ago

Sorry, I meant to mark this as a feature request.

#2 Updated by Woody Gilk about 5 years ago

  • Status changed from New to Assigned
  • Assignee set to Woody Gilk
  • Target version set to v3.0.0

#3 Updated by Woody Gilk about 5 years ago

  • Status changed from Assigned to Closed
  • Resolution set to fixed

#4 Updated by Isaiah DeRose-Wilson about 5 years ago

  • Status changed from Closed to Feedback

I wonder if converting dashes to underscores is actually a good idea. I was just working with an SEO company on a new site I built based on Kohana 3, and they gave it bad marks because it allowed duplicate content. Basically they say it's a bad idea if you have two urls (one with underscores, and one with dashes) that point to the same content without doing a redirect. I'm not really sure how much this matters, a lot of times SEO people like you come up with random problems to make it look like they are doing their job, however it seems like this could cause a few problems. Maybe it would be best to provide an option to use underscores or dashes? Not that I really want more config options through...

#5 Updated by Jonathan Geiger about 5 years ago

From an SEO perspective, I think as long as you keep the internal URLs consistent (always use dashes or underscores) it should be fine. Search engines don't actively test for duplicate content by replacing dashes with underscores.

The reason that this patch is necessary is because the general consensus—as far as the dashes-vs-underscores debate—is that dashes are slightly better from an SEO perspective (not to mention that they are easier to type). However, without the patch it's impossible to have wildcard URLs with dashes in them that you can expect to be routed to PHP classes and methods.

Additionally, if there were underscores in the URL it wouldn't be difficult to overload Kohana_Controller to redirect to the same URL but with dashes. However, this should occur on a per-app-basis; Kohana has always been more about flexibility than convention.

#6 Updated by Isaiah DeRose-Wilson about 5 years ago

Jonathan Geiger wrote:

From an SEO perspective, I think as long as you keep the internal URLs consistent (always use dashes or underscores) it should be fine. Search engines don't actively test for duplicate content by replacing dashes with underscores.

Correct, but if someone types in (or links to your site with) the 'wrong' url they should get a 404 page (or be redirected) instead of having either underscores and dashes work.

The reason that this patch is necessary is because the general consensus—as far as the dashes-vs-underscores debate—is that dashes are slightly better from an SEO perspective (not to mention that they are easier to type).

I'm not against this patch, I'm just afraid that we might be creating a bigger problems than we are fixing.

#7 Updated by Woody Gilk about 5 years ago

  • Target version changed from v3.0.0 to v3.1.0
  • Resolution deleted (fixed)

I am going to undo this patch and move it to a 3.1 target. If you need to implement this now, you can extend Request or Route.

http://github.com/kohana/core/commit/3f6919bee0dfdb2c0c4d26beb767c0b126fe9dd6

#8 Updated by Paul Banks about 5 years ago

For what it's worth I think both SEO points discussed here are important.

Hyphens are more user friendly (which should be the main reason to use them!) than any other word separation in urls and are likely to be marginally preferred by search algorithms. They appear to do better based on my SEO experiments anyway.

The duplicate content issue however is genuine though and is not just (or not only!) a money making diagnosis by an SEO team! It goes beyond hyphen vs underscore though. For example you may have a non standard index action which would make controller/ and controller/action the same content (not ideal) or even more complex rules such as blog/post-title and blog/15 pointing to the same content. The duplicate content issue effects all of these.

To some extent though it is not necessarily the framework's job to do something about this. If it was there would be a good argument for not being able to define default actions or multiple routes that can get to the same place as above. My approach based on experiences with very large sites where it is impossible to keep track of all the in-bound links and slight URL variations that somehow crop up, is to have a single place where you define the correct URL for any given page and then, if an incorrect url is used, redirect (and it HAS to be a 301 redirect) to the correct URL.

For this reason, I am pleased with the new routing in KO3 because it allows me to define my routes and then get a consistent link back to a page using the uri() method with parameters so I don't accidentally link to blog/title in one place and blog/id in another. I am considering for my own applications, extending Kohana_Controller to check for the 'correct' URL after routing and handle the 301 redirects automatically. Note that I see this as part of the application design and specification (no duplicate content) rather than something I expect Kohana to look after for me.

In summary then, I think the vast majority would welcome the original patch suggested here as it is easy and user/developer friendly. If developers are unaware of duplicate content issues, the chances are they will break the rules elsewhere in their application anyway. I think it is beyond the scope of a framework (especially one which tries to be so open and unrestrictive) to enforce any kind of policy on URL uniqueness. The only way you will be successful there is to not allow custom routing at all. That of course is ridiculous.

I hope this is a helpful contribution to the discussion.

#10 Updated by Woody Gilk almost 5 years ago

  • Category set to Core

#11 Updated by Kent Davidson over 4 years ago

To add my $0.02 adding <link rel="canonical" ... /> tag to pages should eliminate any worries with SEO and URLs. It's honored by Google and everyone else who matters.

For that matter, I can create billions of identical pages:

http://dev.kohanaphp.com/issues/1942?1
http://dev.kohanaphp.com/issues/1942?2
http://dev.kohanaphp.com/issues/1942?3

And they're all the same. Hence the canonical tag tells a search engine what the canonical name for a page should be.

#12 Updated by Torleif Berger over 4 years ago

How about just removing the dashes instead?

So that about-us/mission-statement would point to Controller_AboutUs with action action_missionstatement or something like that. At least for the controller, cause if I have understood correctly underscores kind of signifies directory levels or something?

#13 Updated by Woody Gilk about 4 years ago

  • Status changed from Feedback to Closed
  • % Done changed from 0 to 100

Applied in changeset commit:"3f6919bee0dfdb2c0c4d26beb767c0b126fe9dd6".

#14 Updated by Kiall Mac Innes about 4 years ago

  • Status changed from Closed to Feedback
  • % Done changed from 100 to 0

Apologies - I batch closed a few tickets by mistake. Reverting.

#15 Updated by Karen Kratyan about 4 years ago

http://github.com/kratkar/core/commit/9c53083f2d3f017c8f226d9256b956ef55518151
http://github.com/kratkar/core/commit/22c6065d4880666fb23620b182314faaf23cacca

Example (/about-us/mission-statement):

Route::set('about-us', 'about-us(/<action>(/<id>))')
    ->defaults(array(
        'controller' => '_mycontroller_about_us',
        'action'     => 'index',
    ));
Class Mycontroller_About_Us extends Controller
{
   public $map_call = array(
       'mission-statement' => 'myaction_mission_statement'
   );

   public function myaction_mission_statement()
   {
      echo 'myaction_mission_statement';
   }
}

#16 Updated by Jeremy Bush about 4 years ago

  • Assignee changed from Woody Gilk to Sam de Freyssinet

#17 Updated by Sam de Freyssinet about 4 years ago

I'm not convinced myself. The route definitions should handle this conversion within the definition itself. The use of underscores and dashes within URI's are both legal, regardless of convention and ultimately this is adding additional overhead to what is a rapidly growing routing process. It is relatively painless to extend the router to handle this case if required, but I think best practice would be to define routes correctly.

#18 Updated by Jeremy Bush about 4 years ago

  • Status changed from Feedback to Closed
  • Resolution set to wontfix

I tend to agree.

Also available in: Atom PDF