#153 open
Zach Dennis

within should support xpath

Reported by Zach Dennis | February 3rd, 2009 @ 10:46 PM | in 0.5

The current #within method supports only css selectors. When trying to use xpath it still tries to use CSS. For example, in scope.rb the scoped_dom method always uses #css_at:


    def scoped_dom
      Webrat::XML.css_at(@scope.dom, @selector)
    end

There are some dom traversal techniques in which xpath is better suited than css (such as traversing down the dom and then back up which css can't do). xpath can usually be spotted by checking if the selector string starts with backslashes and since backslashes aren't used with css selectors AFAIK there shouldn't be any confusion or edge cases.

WDYT?

Comments and changes to this ticket

  • Bryan Helmkamp

    Bryan Helmkamp February 8th, 2009 @ 11:41 PM

    • Assigned user set to “Bryan Helmkamp”
    • State changed from “new” to “open”
    • Milestone set to 0.5

    Zach,

    I agree it should be supported. At first glance, I'm wondering if we should add within_xpath instead of overloading the within API to take either style. On the whole, I explicit to implicit in the Webrat API, and am trying to move in that direction.

    In either form, I'd like to get this into the 0.5 release.

    WDYT?

    -Bryan

  • Zach Dennis

    Zach Dennis February 9th, 2009 @ 08:33 PM

    I agree with you Bryan, being explicit seems like a better route to go. If it becomes a need to have a single consolidated interface it can always be revisited that later.

  • tomtt

    tomtt March 3rd, 2009 @ 06:53 AM

    The problem with just adding within_xpath is that methods like click_link_within will still not be able to use xpath. I do not know the codebase very well, but it seems to me that by allowing the selector to be a string (assuming it is a css selector) or an XPath object and passing that selector all the way down, then all that needs to be done is not convert the selector to an XPath object if it already is one. Am I right?

  • tomtt

    tomtt March 3rd, 2009 @ 08:20 AM

    Here is a duck punch that solved my immediate need. For a real patch, it should check if selector is an XPath object.

    
    # Webrat does not accept xpath selectors. This is a quick hack to make
    # it work for me (assumes xpaths start with /, everything else is css)
    
    module Webrat
      class Scope
        protected
    
        def scoped_dom
          if @selector =~ /^\.?\/\/?/
            Webrat::XML.xpath_at(@scope.dom, @selector)
          else
            Webrat::XML.css_at(@scope.dom, @selector)
          end
        end
      end
    end
    
  • tomtt

    tomtt March 4th, 2009 @ 09:07 AM

    Another duckpunch which is slightly more elegant I believe. It tries to call css_at with the selector, if it raises a syntax error the selector is passed to xpath_at. If that fails then the original exception is raised.

    My knowledge of css vs xpath selectors is limited so this may be a Very Bad Idea.

    
    # Webrat does not accept xpath selectors. This is a quick hack to make
    # it work for me (assumes xpaths start with //, everything else is css)
    
    module Webrat
      class Scope
        protected
    
        def xpath_scoped_dom
          Webrat::XML.xpath_at(@scope.dom, @selector)
        end
    
        def scoped_dom
          begin
            Webrat::XML.css_at(@scope.dom, @selector)
          rescue Nokogiri::CSS::SyntaxError, Nokogiri::XML::XPath::SyntaxError => e
            begin
              # That was not a css selector, mayby it's an xpath selector?
              xpath_scoped_dom
            rescue
              # Raise original css syntax error if selector is not xpath either
              raise e
            end
          end
        end
      end
    end
    
  • linoj

    linoj March 16th, 2009 @ 01:10 PM

    tomtt's patch is working for me so far. You might change the comment at the top though (it doesnt assume //)

  • tomtt

    tomtt March 16th, 2009 @ 02:22 PM

    @linoj: you are right, i had fixed it in my code, but thought it not worth another ticket update. But here you go:

    
    # Webrat does not accept xpath selectors. This is a quick hack to make
    # it work for me. It tries to call css_at with the selector, if it
    # raises a syntax error the selector is passed to xpath_at. If that
    # fails then the original exception is raised.
    
    module Webrat
      class Scope
        protected
    
        def xpath_scoped_dom
          Webrat::XML.xpath_at(@scope.dom, @selector)
        end
    
        def scoped_dom
          begin
            Webrat::XML.css_at(@scope.dom, @selector)
          rescue Nokogiri::CSS::SyntaxError, Nokogiri::XML::XPath::SyntaxError => e
            begin
              # That was not a css selector, mayby it's an xpath selector?
              xpath_scoped_dom
            rescue
              # Raise original css syntax error if selector is not xpath either
              raise e
            end
          end
        end
      end
    end
    
  • tomtt

    tomtt March 18th, 2009 @ 09:01 PM

    I have forked webrat on github and committed this fix for it. It has been working well for me in practice. The only caveat I can think of is if you'd want to use an xpath expression that is also a valid css selector. I can not currently think of one though. If somebody can give me an example of an html fragment and a string that can be both a valid css selector and a valid xpath but that should produce different results then I will find a way to code for that as well.

    http://github.com/tomtt/webrat/t... 61d859b2ffcb208d6b92d4d2f00b1a880b7899a8

  • Tom Adams

    Tom Adams December 1st, 2009 @ 01:52 PM

    • Tag changed from bug, within, xpath to bug, pull, within, xpath

    Here's tomtt's patch fixed for today's master: http://github.com/dxw/webrat/tree/scoped_dom

  • tomtt

    tomtt December 2nd, 2009 @ 08:26 AM

    Thanks for that, there is one minor tweak I made: just raising does not halt the exception chain. I have updated my github webrat fork accordingly:
    http://github.com/tomtt/webrat/tree/xpath_fix
    http://github.com/tomtt/webrat/commit/bbdd1c1cd6cd374afe31255c5fcbb...

  • tomtt

    tomtt December 2nd, 2009 @ 08:45 AM

    In fact, that was probably not the right thing to do. Sorry, please ignore that commit, you need to reraise the original exception to not break apps that catch it. I have added a spec to check that the correct type of error is raised.

    http://github.com/tomtt/webrat/commit/2aeb68899edd7abdc77fedb7c552b...

  • kwerle

    kwerle January 5th, 2010 @ 07:14 PM

    See also
    http://objectliteral.blogspot.com/2009/09/clicking-on-links-in-webr...

    For 0.6.0 I am using

    module Webrat
    class Scope

    {mkd-extraction-90c8d48709e810f68642dca8ed82a220}

    end end

  • kwerle

    kwerle January 5th, 2010 @ 07:15 PM

    Sigh. Preview does not work. How about plain text:

    module Webrat
    class Scope

    protected
    
    def xpath_scoped_dom
      #HaveXpath.new(@selector).matches.first
      hs = have_xpath(@selector)
      hs.matches(@scope.dom)
      #Webrat::XML.xpath_at(@scope.dom, @selector)
    end
    
    def scoped_dom
      begin
        @scope.dom.css(@selector).first
        #Webrat::XML.css_at(@scope.dom, @selector)
      rescue Nokogiri::CSS::SyntaxError => e#, Nokogiri::XML::XPath::SyntaxError => e
        begin
          # That was not a css selector, mayby it's an xpath selector?
          xpath_scoped_dom
        rescue Exception => e2
          # Raise original css syntax error if selector is not xpath either
          raise e2
        end
      end
    end
    

    end end

  • dannyhawkins (at me)

    dannyhawkins (at me) January 12th, 2010 @ 11:49 AM

    I've just added the following:

    def scoped_dom

      if @selector =~ /^\.?\/\/?/
        @scope.dom.xpath(@selector).first
      else
        @scope.dom.css(@selector).first
      end
    end
    

    Which seems to work very well. Any reason why this shouldn't be used?

  • Tom Adams

    Tom Adams January 12th, 2010 @ 12:37 PM

    The reason is that it's wrong. XPath expressions don't all start with a '/' or a '.'.

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

Pages