Permalink
Browse files

modified current_page? to ignore extra parameters unless specified in…

… options

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#805 state:committed]
  • Loading branch information...
1 parent 2c7abe1 commit ef9b6b5cba08f13dcbf7095226b78aaf22df13f7 @eandrejko eandrejko committed with NZKoz Oct 26, 2008
Showing with 24 additions and 3 deletions.
  1. +14 −3 actionpack/lib/action_view/helpers/url_helper.rb
  2. +10 −0 actionpack/test/template/url_helper_test.rb
@@ -499,14 +499,17 @@ def mail_to(email_address, name = nil, html_options = {})
# True if the current request URI was generated by the given +options+.
#
# ==== Examples
- # Let's say we're in the <tt>/shop/checkout</tt> action.
+ # Let's say we're in the <tt>/shop/checkout?order=desc</tt> action.
#
# current_page?(:action => 'process')
# # => false
#
# current_page?(:controller => 'shop', :action => 'checkout')
# # => true
#
+ # current_page?(:controller => 'shop', :action => 'checkout', :order => 'asc)
+ # # => false
+ #
# current_page?(:action => 'checkout')
# # => true
#
@@ -515,10 +518,18 @@ def mail_to(email_address, name = nil, html_options = {})
def current_page?(options)
url_string = CGI.escapeHTML(url_for(options))
@pixeltrix

pixeltrix Nov 14, 2008

Owner

I may be being dense here but shouldn’t this be CGI.unescapeHTML otherwise we’re double escaping the generated url and it stands no chance of being equal to the request uri if there’s an ampersand in the url.

request = @controller.request
+ # We ignore any extra parameters in the request_uri if the
+ # submitted url doesn't have any either. This lets the function
+ # work with things like ?order=asc
+ if url_string.index("?")
+ request_uri = request.request_uri
+ else
+ request_uri = request.request_uri.split('?').first
+ end
if url_string =~ /^\w+:\/\//
- url_string == "#{request.protocol}#{request.host_with_port}#{request.request_uri}"
+ url_string == "#{request.protocol}#{request.host_with_port}#{request_uri}"
else
- url_string == request.request_uri
+ url_string == request_uri
end
end
@@ -258,6 +258,16 @@ def test_link_unless_current
assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show")
+ @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc")
+ @controller.url = "http://www.example.com/weblog/show"
+ assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
+ assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show")
+
+ @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc")
+ @controller.url = "http://www.example.com/weblog/show?order=asc"
+ assert_equal "<a href=\"http://www.example.com/weblog/show?order=asc\">Showing</a>", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
+ assert_equal "<a href=\"http://www.example.com/weblog/show?order=asc\">Showing</a>", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=asc")
+
@controller.request = RequestMock.new("http://www.example.com/weblog/show")
@controller.url = "http://www.example.com/weblog/list"
assert_equal "<a href=\"http://www.example.com/weblog/list\">Listing</a>",

3 comments on commit ef9b6b5

cs replied Oct 27, 2008

Thank you very much! This is really useful ;)

Member

NZKoz replied Nov 14, 2008

@pixeltrix: That code has been there a while, it’s not new with this changeset.

But anyway, the query-string is ignored now, so it shouldn’t matter right?

The source of it is:

http://github.com/rails/rails/commit/3dc7f7603785c1173df5c24f8b76323f7a795443

Owner

pixeltrix replied Nov 16, 2008

I think it’s been broken for a while. :-)

The query string is only ignored if the url being checked doesn’t have a query string of its own, so it fails it two circumstances – once when passing in a hash where the current request has multiple params and also when passing in explicit urls with multiple params.

I’ve created a ticket with a patch at:

http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1385-fix-current_page-to-work-with-multiple-parameters-in-the-query-string

Please sign in to comment.