Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misreporting html errors with radio buttons and checkboxes if the text is capitalized with css. #236

Open
keathley opened this issue May 12, 2017 · 1 comment

Comments

@keathley
Copy link
Member

This is something that I just noticed. Its pretty common to set a radio button inside a label like so:

<label>
  <input type="radio">
  Some radio
</label>

In these cases Wallaby shouldn't complain about the label missing a for but it does. The reason why is kinda interesting.

If the label has some css on it like so text-transform: capitalize then in the browser the text will appear as "Some Radio". If you then try to click the radio using something like: click(radio_button("Some Radio")) you'll get the "label is missing a for" error. The reason you get this error is because our xpath query is case sensitive and the xpath looks at whats actually in the DOM. So because the label doesn't match anything we fall into the error flow. We then try to be helpful and validate the HTML. We do that using a query like: css("label", text: "Some Radio"). When the css query is executed it correctly finds the label! This is because when we check text with css we use the text webdriver endpoint which returns rendered text instead of the text thats actually in the DOM. Because our validate html check is finding the label, and because the label doesn't have a for attribute Wallaby is misreporting this as an HTML bug.

There are a couple of ways to fix this. One is to not use the same validate_html logic for checkboxes and radio buttons. This is similar to what we do for buttons. Another would be to normalize all of the casing across our api. This is somewhat tricky because it would mean doing a translation in the xpath queries. We would have to do it in xpath since we can't change what the webdriver returns from text.

The other option is something I've been talking about for a long while but its translating ALL queries into xpath under the hood. In this case it would actually have solved this issue since we'd always be using the same querying operations. That seems like a more involved fix though.

Somewhat related it would be nice to have a specific error for when you search for something that has text and you accidentally get casing wrong or misspell the text. I'm not sure if there is a good way to do that or not but I'd love to find a solution that could accommodate some helpful hints like that.

@RobinClowers
Copy link

I suspect I ran into a similar problem with this code: find(session, button("SIGN OUT")). The button has type="button", yet I get this error:

** (Wallaby.QueryError) The text 'SIGN OUT' matched a button but the button has an invalid 'type' attribute.

     You can fix this by including `type="[submit|reset|button|image]"` on the appropriate button.

@keathley keathley added this to the 1.0 milestone Jul 11, 2018
ShahneRodgers added a commit to ShahneRodgers/wallaby that referenced this issue Oct 10, 2022
This fixes elixir-wallaby#236 by using the validate_html logic, that was previously
used exclusively by buttons, for both checkboxes and radio buttons.
This changes the error message Wallaby reports from complaining about
a missing label to one that simply states the label does not exist.

This also extends the HTML validation for buttons so that helpful hints
can still be provided, and adds a new error message for when the element
isn't found but the CSS query finds the label.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants