Problem/Motivation

It appears that due to #3084798 in the clientside_validation module, that custom:

other__required_error: 'Some error'

properties aren't working with clientside validation anymore. I believe that this may be related to #states not being used to hide / show the other option in these element types.

Steps to reproduce

  1. Enable webform 6.x, clientside_validation 3.x, and webform_clientside_validation.
  2. Add a radios-other and checkboxes-other element to a webform.
  3. Configure a custom required error message for each element.
  4. Select 'other' for each and submit the form.
  5. Observe that the validation message for both elements is "This field is required" and not the custom message.

Proposed resolution

TBD

Remaining tasks

TBD

User interface changes

TBD

API changes

TBD

Data model changes

TBD

Thanks!

CommentFileSizeAuthor
#13 3217704-13.patch5.18 KBjrockowitz
#10 3217704-9.patch5.24 KBjrockowitz
#8 3217704-8.patch6.33 KBjrockowitz
#5 3217704-custom-other-error-messages-5.patch5.98 KBluke.leber
#4 3217704-test-only-fail.patch4.63 KBluke.leber
#2 webform.webform._3217704_custom_other_error_mess.yml5.04 KBpaulocs

Issue fork webform-3217704

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Luke.Leber created an issue. See original summary.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
StatusFileSize
new webform.webform._3217704_custom_other_error_mess.yml5.04 KB

Here is a webform example to replicate the issue.
I see if I provide a patch later.

jrockowitz’s picture

jrockowitz commented
Version: 6.x-dev » 8.x-5.x-dev

Let's also fix this issue in 8.x-5.x. I think something like the below JS solution as a Drupal.behavior could go in modules/webform_clientside_validation/js/webform_clientside_validation.ife.js

$('.webform-checkboxes-other-input input, .webform-radios-other-input input').each(function() {
  const $this = $(this);
  const attr = $this.attr('data-msg-required');
  const webform_attr = $this.attr('data-webform-required-error');
  if (typeof attr === 'undefined' && typeof webform_attr !== 'undefined') {
    $this.attr('data-msg-required', webform_attr);
  }
});
luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented
StatusFileSize
new 3217704-test-only-fail.patch4.63 KB

The problem seems to also affect select_other element types.

Attaching fail-patch to prove there is a problem (let's hope the D.O. test bot agrees!).

I had to:

  1. Include the inline_form_errors module to ensure that the javascript is loaded on the test page.
  2. Add *-other elements to the existing test webform.
  3. Add some assertions to check the custom error messaging.

Another unrelated point of interest:

I don't think that this line runs any assertions.

    $assert_session->waitForText('tableselect_radios field is required.');

waitForText seems to just return FALSE if the text isn't found.

I forgot to ask, are patches or issue forks preferred for the Webform module at this time?

*after-run edit...

It looks like the test runner pulls in the 8.x-1.x branch for clientside_validation even though the composer.json file clearly states ~3.0 in the require-dev section...so I'm not sure what's going on.

https://dispatcher.drupalci.org/job/drupal8_contrib_patches/71928/console

22:49:26   - Locking drupal/clientside_validation (1.2.0)
22:49:26   - Locking drupal/clientside_validation_jquery (1.2.0)

May have to ask in the infrastructure / testing channel to figure out why this is.

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented
StatusFileSize
new 3217704-custom-other-error-messages-5.patch5.98 KB

Uploading patch that should fix the test that was broken in #4.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented

Working on it...

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Active » Reviewed & tested by the community

Patch looks good! It has test coverage and it fixes the problem properly.

jrockowitz’s picture

jrockowitz commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 3217704-8.patch6.33 KB

Attached is minor cleanup of the patch.

I am not sure we can assume that the inline_form_error.module is enabled with clientside validation. Maybe via backend code, we set the correct attribute.

Status: Needs review » Needs work

The last submitted patch, 8: 3217704-8.patch, failed testing. View results

jrockowitz’s picture

jrockowitz commented
Status: Needs work » Needs review
StatusFileSize
new 3217704-9.patch5.24 KB

Attached is what I hope is a simpler fix that does not rely on the JS or the inline_form_errors.module. BTW, Having the test coverage and an example helped a lot.

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented

I really like the approach in #10. This even covers users (like us) whose custom clientside validation integration work predates the webform_clientside_validation module.

It seems to work perfectly with our custom integration as well as with the stock webform modules.

+1 RTBC from me!

luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented

Actually, one small nitpick. There seems to be a duplicated comment in the test.

@@ -61,6 +62,31 @@ class WebformClientSideValidationJavaScriptTest extends WebformWebDriverTestBase
...
+    // Check that custom 'other' error messages work.
+    $this->drupalGet('/webform/test_clientside_validation');
+    $page->findById('edit-select-other-select')->selectOption('_other_');
+    $page->findById('edit-radios-other-radios-other-')->selectOption('_other_');
+    $page->findById('edit-checkboxes-other-checkboxes-other-')->check();
+    $this->submitForm([], 'Submit');
+
+    // Check that custom 'other' error messages work.   <-- Duplicate
+    $custom_errors = [
jrockowitz’s picture

jrockowitz commented
StatusFileSize
new 3217704-13.patch5.18 KB
luke.leber’s picture

luke.leber
Primary language English
Location Pennsylvania
commented
Status: Needs review » Reviewed & tested by the community

#13 looks perfect to me.

jrockowitz’s picture

jrockowitz commented
Status: Reviewed & tested by the community » Fixed

  • jrockowitz authored d5494c6 on 8.x-5.x 
    Issue #3217704 by Luke.Leber, jrockowitz, paulocs: Custom 'other error'...

  • jrockowitz authored d5494c6 on 6.x 
    Issue #3217704 by Luke.Leber, jrockowitz, paulocs: Custom 'other error'...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.