
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 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.
-
-
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.
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.