mojolicious / mojo

:sparkles: Mojolicious - Perl real-time web framework
https://mojolicious.org
Artistic License 2.0
2.66k stars 576 forks source link

Inconsistent behaviour of error class propagation of input fields #1487

Closed Skeeve closed 3 years ago

Skeeve commented 4 years ago

Steps to reproduce the behavior

Please compare the output of this (# 1)

  %= label_for "owner" => begin
    Owner
    %= text_field 'owner' => ( id => 'owner' )
  % end

To the output of this (# 2)

  %= label_for "owner_ID" => begin
    Owner
    %= text_field 'owner' => ( id => 'owner_ID' )
  % end

and this (# 3)

  %= label_for "owner" => begin
    Owner
    %= text_field 'owner'
  % end

in case of a validation error on the field.

When id eq name (# 1), label gets the error class:

  <label for="owner" class="field-with-error">
    Owner
    <input class="field-with-error" id="owner" name="owner" type="text" value="" data-kpxc-id="owner">
</label>

When id ne name (# 2), label does not get the error class:

  <label for="ownerX">
    Owner
    <input class="field-with-error" id="ownerX" name="owner" type="text" value="" data-kpxc-id="ownerX">
</label>

When id is missing, label gets the error class:

<label class="field-with-error" for="owner">
    Owner
    <input class="field-with-error" name="owner" type="text" value="" data-kpxc-id="kpxcpw342845639">
</label>

Expected behavior

While I understand that the "for" attribute refers to the "id" attribute of the labeled field, I do not understand why id and name have to be identical in order to propagate the class to the label. I expected the label to get the class as well.

I also do understand that, in case of the nested input element, there shouldn't be a "for" attribute, but unfortunately the class does not get propagated up to the surrounding label at all.

So what I would expect when there was a validation issue on a field:

  1. Every label which is "for" the field should get the error class, regardless of whether or not 'name' and ' id' are equal.
  2. A surrounding label should get the error class.

Actual behavior

label does not get the error class when there is no "for" attribute or the field's "id" does not match its "name".

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Skeeve commented 3 years ago

Seems no one cares or can explain :(

kraih commented 3 years ago

Or nobody agrees with your reasoning.

kraih commented 3 years ago

But i guess considering that there are no links to specific spec sections backing up your reasoning, it could also be that nobody took a closer look.

Skeeve commented 3 years ago

Okay…

So here's a reference which made me think it's automatic.

https://docs.mojolicious.org/Mojolicious/Plugin/TagHelpers

For fields that failed validation with "validation" in Mojolicious::Plugin::DefaultHelpers the field-with-error class will be automatically added through "tag_with_error", to make styling with CSS easier.

kraih commented 3 years ago

When i say spec, i mean like the HTML5 spec.

Skeeve commented 3 years ago

What does HTML5 specs to do with this.

You know what…

Maybe it's due to the fact that I'm not a native speaker, but a lot of the Mojolicious Community appears quite arrogant to me. This is not the first case.

I will close the issue now and simply forget about all this.

Bye…

kraih commented 3 years ago

We don't make decisions about HTML semantics arbitrarily, we follow the specs in all our APIs. If you don't like that then that's on you.

Skeeve commented 3 years ago

Still I do not see where HTML5 is related. It's just about tag helpers and validaton.

shadowcat-mst commented 3 years ago

@Sheeve because the error behaviour is based on the HTML5 specification.

If you want to make a case for this being a bug, you need to do so based on https://www.w3.org/TR/html52/sec-forms.html

I think it may actually be possible to do so, but you're going to need to understand how HTML5 works to make that argument, and that's not arrogance, that's just "in order to argue for something being a bug, you need to understand what standard the code's implementing". Also reading RFCs is fun :P

Skeeve commented 3 years ago

https://www.w3.org/TR/html52/sec-forms.html#the-label-element

Example #2 on the page says:

Note that the id attribute is required to associate the for attribute, while the name attribute is required so the value of the input will be submitted as part of the form.

Please also note that in my example #2 the id of the element matches the "for" of the label.

If that doesn't qualify as a bug found, I don't know.