#231 awaiting-patch
Tom Stuart

LinkLocator#link_element "best match" behaviour is broken

Reported by Tom Stuart | April 25th, 2009 @ 06:25 AM

...or at least surprising.

Since November, link_element has worked by finding all link elements whose text content, HTML content or title attribute matches the locator value, and then returning the element whose text content is the shortest.

Namely: there are now several criteria for deciding which elements are potential candidates, but only one of those criteria is used when deciding which element is the "best" match.

This change broke a whole bunch of stuff for me when I upgraded Webrat. In particular I have many pages that contain links with no text content (e.g. images) but whose HTML content (e.g. alt tags) often matches the locator value. These no-text-content links are never what I want to click on with click_link (and if they were I'd give the locator a value like alt="...") but they invariably get picked by link_element because they have the shortest (i.e. completely empty) text content.

The attached patch fixes this behaviour, or at least makes it more predictable and consistent: we now keep track of the matching content for each link, and use the actual length of that content when scoring the match candidates. This still might not be what everyone wants but it's much closer to being correct. I've left the old API in place in case anyone was depending on it, but otherwise matches_text?, matches_id? and matching_links can be removed.

Comments and changes to this ticket

  • Tom Stuart

    Tom Stuart April 25th, 2009 @ 07:11 AM

    Whoops, I see now that a hash isn't suitable, since the stability of min meant that links occurring earlier in the page always won in a tiebreak, but that information gets lost when the links are stored as hash keys.

    I've attached a new patch which uses an array of pairs instead, thereby preserving the expected tiebreaking behaviour.

  • Tom Stuart
  • gaffo

    gaffo May 10th, 2009 @ 10:09 PM

    • Tag changed from click_link, enhancement, patch, regression to click_link, enhancement, patch, regression, test!, verify
    • State changed from “new” to “awaiting-patch”

    Tom, Thanks for the patch. However, some tests showing what it does and proving that it works will keep this from regressing. It's not mandatory, but it helps speed things up.

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