#62 open
rbpandey

Field.to_param should check to see if Merb is defined before parsing query parameters

Reported by rbpandey | November 17th, 2008 @ 03:50 PM

I changed the else block to an elsif for Merb query param parsing. I also added a dumb else block to just return the key_and_value local variable.

def to_param

  return nil if disabled?

  key_and_value = "#{name}=#{escaped_value}"

  if defined?(CGIMethods)
    CGIMethods.parse_query_parameters(key_and_value)
  elsif defined?(ActionController::AbstractRequest)
    ActionController::AbstractRequest.parse_query_parameters(key_and_value)
  elsif defined?(::Merb)
    ::Merb::Parse.query(key_and_value)
  else
    key_and_value
  end
end

Comments and changes to this ticket

  • rbpandey

    rbpandey November 18th, 2008 @ 02:40 PM

    The else clause needed to hand back a hash so that Mechanize could do it's thing.

    def to_param return nil if disabled?

    key_and_value = "#{name}=#{escaped_value}"

    if defined?(CGIMethods)

    CGIMethods.parse_query_parameters(key_and_value)
    
    

    elsif defined?(ActionController::AbstractRequest)

    ActionController::AbstractRequest.parse_query_parameters(key_and_value)
    
    

    elsif defined?(::Merb)

    ::Merb::Parse.query(key_and_value)
    
    

    else

    {name => escaped_value}
    
    

    end end

  • rbpandey

    rbpandey November 18th, 2008 @ 02:42 PM

    Just going to attach my lib/webrat/core/field.rb so that you don't have problems reading the method, given my formatting woes.

  • Bryan Helmkamp

    Bryan Helmkamp November 18th, 2008 @ 08:49 PM

    • State changed from “new” to “resolved”

    (from [df9d8179c0d8ed2061c5f91a9dd55d8c86fc5e26]) Field#to_param should return a hash when Merb is not defined for Mechanize support [#62 state:resolved] http://github.com/brynary/webrat...

  • Bryan Helmkamp

    Bryan Helmkamp November 18th, 2008 @ 08:50 PM

    Thanks for the patch. I've applied it.

  • rbpandey

    rbpandey December 11th, 2008 @ 05:32 PM

    Hey Bryan,

    As an update to this, I was testing some form posts with Webrat and Mechanize and found that the to_param method shouldn't be escaping the form field value when using Mechanize. So line 86 of http://github.com/brynary/webrat...

    should look like :

    
     { name => @value }
    

    and the whole to_param method should look like:

    
       def to_param
          return nil if disabled?
          
          case Webrat.configuration.mode
          when :rails
            ActionController::AbstractRequest.parse_query_parameters("#{name}=#{escaped_value}")
          when :merb
            ::Merb::Parse.query("#{name}=#{escaped_value}")
          else
            { name => @value }
          end
        end
    

    Thx, Rus

    ps - i should have some time soon to help round out the test suite with respect to Mechanize.

  • rbpandey

    rbpandey December 11th, 2008 @ 05:37 PM

    as a follow-up to my last update, essentially we should be letting Mechanize handle any escaping of values, as needed.

  • Bryan Helmkamp

    Bryan Helmkamp December 15th, 2008 @ 02:34 PM

    • State changed from “resolved” to “open”
  • gaffo

    gaffo June 3rd, 2009 @ 10:12 AM

    • Tag set to bug, featurerequest, stale

    rbpandey,
    It's been quite a while, is this still an issue? If so could you supply an updated patch that I can take a look at?

    Thanks

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Ruby Acceptance Testing for Web applications.

Shared Ticket Bins

People watching this ticket

Attachments

Referenced by

Pages