gma / nesta

File Based CMS and Static Site Generator
http://nestacms.com
MIT License
902 stars 121 forks source link

current_item? helper fails when mounted at /path #112

Closed gma closed 4 months ago

gma commented 12 years ago

If you mount Nesta on a path other than / the current_item? helper fails as it compares the page's path relative to the content/pages folder with the value returned by request.path.

Reported by @mrnosal.

ms commented 12 years ago

To save other people the time I took to understand exactly what was supposed to happen that wasn't, the HTML for the menu should look like this:

            <li class='current'>
              <a href='/'>Home</a>
            </li>
            <li>
              <a href='/blog'>Blog</a>
            </li>

if you are currently on the home page. If mounted at a subpath, no menu item has this class.

The place the class is defined is in lib/nesta/navigation.rb and it is used by display_menu_item, itself called by display_menu (and ultimately by haml in the views).

The reason this happens is that current_item? simply compares the item.path—from the menu.txt file and relative to the root URI (e.g. http://example.com/nesta/)—to the request.path, which according to the Rack document is the script name concatenated with the path_info, which are

SCRIPT_NAME The initial portion of the request URL's "path" that corresponds to the application object, so that the application knows its virtual "location". This may be an empty string, if the application corresponds to the "root" of the server.

PATH_INFO The remainder of the request URL's "path", designating the virtual "location" of the request's target within the application. This may be an empty string, if the request URL targets the application root and does not have a trailing slash.

(from a Python mailing list about some web app

Therefore the solution seems simple: use request.path_info instead of request.path. I'm gonna test that and more importantly try to come up with a unit test that corresponds to the bug. Not sure how yet though.

ms commented 12 years ago

I can confirm changing path to path_info solves the problem.

ms commented 12 years ago

In a test, it is possible to passe a specific environment, in particular a specific SCRIPT_NAME and PATH_INFO like so:

get '/url_for', {}, 'SCRIPT_NAME' => '/bar'

(From a sinatra app on github.)

So if somehow the config.ru has the correct url mappings, it should be relatively easy to test whether the right menu element has the current class. I'm not sure exactly how to do that yet though.

gma commented 12 years ago

Awesome – thanks for doing the research.

gma commented 4 months ago

This was fixed back in 2018 in commit eab9373.