[release-10.1] Fix form validation (#4227)

The old validation mechanism was not compatible with Bootstrap 5 and didn't work properly with Bootstrap 3 either. The new uses the JS validation API. It is theme-independent and less intrusive with the caveat that the validation error messages are displayed in a browser-specific style.

module/VuFind/src/VuFindTest/Integration/MinkTestCase.php

@@ -829,6 +829,40 @@ protected function hasElementsMatchingText(Element $page, $selector, $text)
         return false;
     }
 
+    /**
+     * Check that a field content is valid (does not have the :invalid pseudo class).
+     *
+     * @param Element $page     Page element (not currently used)
+     * @param string  $selector CSS selector
+     *
+     * @return void
+     */
+    protected function checkFieldIsValid(Element $page, string $selector): void
+    {
+        $session = $this->getMinkSession();
+        $session->wait(
+            $this->getDefaultTimeout(),
+            "document.querySelector('$selector:invalid') === null"
+        );
+    }
+
+    /**
+     * Check that a field content is invalid (has the :invalid pseudo class).
+     *
+     * @param Element $page     Page element (not currently used)
+     * @param string  $selector CSS selector
+     *
+     * @return void
+     */
+    protected function checkFieldIsInvalid(Element $page, string $selector): void
+    {
+        $session = $this->getMinkSession();
+        $session->wait(
+            $this->getDefaultTimeout(),
+            "document.querySelector('$selector:invalid') !== null"
+        );
+    }
+
     /**
      * Wait for a callback to return the expected value
      *

module/VuFind/tests/integration-tests/src/VuFindTest/Mink/RecordActionsTest.php

@@ -702,32 +702,30 @@ public function testSMS(): void
         // - too empty
         $this->findCssAndSetValue($page, '.modal #sms_to', '');
         $this->clickCss($page, '.modal-body .btn.btn-primary');
-        $this->findCss($page, '.modal .sms-error');
+        $this->checkFieldIsInvalid($page, '.modal #sms_to');
         // - too short
         $this->findCssAndSetValue($page, '.modal #sms_to', '123');
         $this->clickCss($page, '.modal-body .btn.btn-primary');
-        $this->findCss($page, '.modal .sms-error');
+        $this->checkFieldIsInvalid($page, '.modal #sms_to');
         // - too long
         $this->findCssAndSetValue($page, '.modal #sms_to', '12345678912345678912345679');
         $this->clickCss($page, '.modal-body .btn.btn-primary');
-        $this->findCss($page, '.modal .sms-error');
+        $this->checkFieldIsInvalid($page, '.modal #sms_to');
         // - too lettery
         $this->findCssAndSetValue($page, '.modal #sms_to', '123abc');
         $this->clickCss($page, '.modal-body .btn.btn-primary');
-        $this->findCss($page, '.modal .sms-error');
+        $this->checkFieldIsInvalid($page, '.modal #sms_to');
         // - just right
         $this->findCssAndSetValue($page, '.modal #sms_to', '8005555555');
         $this->clickCss($page, '.modal-body .btn.btn-primary');
         $this->waitForPageLoad($page); // wait for form submission to catch missing carrier
-        $this->assertNull($page->find('css', '.modal .sms-error'));
+        $this->checkFieldIsValid($page, '.modal #sms_to');
 
-        $this->unFindCss($page, '.modal .sms-error');
         // - pretty just right
         $this->findCssAndSetValue($page, '.modal #sms_to', '(800) 555-5555');
         $this->clickCss($page, '.modal-body .btn.btn-primary');
         $this->waitForPageLoad($page); // wait for form submission to catch missing carrier
-        $this->assertNull($page->find('css', '.modal .sms-error'));
-        $this->unFindCss($page, '.modal .sms-error');
+        $this->checkFieldIsValid($page, '.modal #sms_to');
         // Send text to false number
         $this->findCssAndSetValue($page, '.modal #sms_to', '(800) 555-5555');
         $this->findCss($page, '.modal #sms_provider option');

themes/bootstrap3/js/common.js

@@ -726,7 +726,12 @@ function getUrlRoot(url) {
   return urlroot;
 }
 
-// Phone number validation
+/**
+ * Phone number validation
+ * @param {String} numID Phone number field ID
+ * @param {String} regionCode Region code
+ * @deprecated See validation.js for replacement
+ */
 function phoneNumberFormHandler(numID, regionCode) {
   var phoneInput = document.getElementById(numID);
   var number = phoneInput.value;
@@ -737,12 +742,10 @@ function phoneNumberFormHandler(numID, regionCode) {
     } else {
       valid = VuFind.translate('libphonenumber_invalid');
     }
-    $(phoneInput).siblings('.help-block.with-errors').html(valid);
-    $(phoneInput).closest('.form-group').addClass('sms-error');
+    phoneInput.setCustomValidity(valid);
     return false;
   } else {
-    $(phoneInput).closest('.form-group').removeClass('sms-error');
-    $(phoneInput).siblings('.help-block.with-errors').html('');
+    phoneInput.setCustomValidity('');
   }
 }
 

themes/bootstrap3/js/validation.js

@@ -0,0 +1,65 @@
+/* global VuFind, isPhoneNumberValid */
+
+VuFind.register('validation', function Validation() {
+  /**
+   * Check field match validity
+   * @param {Event} ev Event
+   * @returns {boolean} Validity
+   */
+  function checkMatchValidity(ev) {
+    const field = ev.target;
+    const matchSelector = field.dataset.match;
+    if (!matchSelector) {
+      return true;
+    }
+    const matchEl = field.form.querySelector(matchSelector);
+    if (matchEl.value !== field.value) {
+      field.setCustomValidity(field.dataset.matchError || '');
+      return false;
+    }
+    field.setCustomValidity('');
+    return true;
+  }
+
+  /**
+   * Check field phone number validity
+   * @param {Event} ev Event
+   * @returns {boolean} Validity
+   */
+  function checkPhoneNumberValidity(ev) {
+    const field = ev.target;
+    if (field.id && field.type === 'tel' && field.dataset.validatorRegion) {
+      const valid = isPhoneNumberValid(field.value, field.dataset.validatorRegion);
+      if (true !== valid) {
+        field.setCustomValidity(typeof valid === 'string' ? valid : VuFind.translate('libphonenumber_invalid'));
+        return false;
+      }
+    }
+    field.setCustomValidity('');
+    return true;
+  }
+
+  /**
+   * Check field validity
+   * @param {Event} ev Event
+   */
+  function checkValidity(ev) {
+    const checks = [checkMatchValidity, checkPhoneNumberValidity];
+    for (const check of checks) {
+      if (!check(ev)) {
+        return;
+      }
+    }
+  }
+
+  /**
+   * Init custom form validation
+   */
+  function init() {
+    document.addEventListener('input', checkValidity);
+  }
+
+  return {
+    init: init
+  };
+});

themes/bootstrap3/less/bootstrap.less

@@ -225,22 +225,6 @@ footer {
 .flex-col { flex: 0 1 100%; }
 .flex-none { flex: none; }
 
-/* ------ Form Errors ------ */
-.has-error {margin-bottom: 0;}
-.sms-error { &:extend(.has-error); }
-.help-block.with-errors {
-  margin: 0;
-  &:first-child {
-    padding-top: @padding-base-vertical;
-  }
-  padding-bottom: @padding-base-vertical;
-  &:empty {padding: 0;}
-
-  ul.list-unstyled {
-    margin: 0;
-  }
-}
-
 /* ------ Admin ------ */
 .form-admin-maintenance .form-control {
   display: inline-block;

themes/bootstrap3/scss/bootstrap.scss

@@ -225,22 +225,6 @@ footer {
 .flex-col { flex: 0 1 100%; }
 .flex-none { flex: none; }
 
-/* ------ Form Errors ------ */
-.has-error {margin-bottom: 0;}
-.sms-error { @extend .has-error; }
-.help-block.with-errors {
-  margin: 0;
-  &:first-child {
-    padding-top: $padding-base-vertical;
-  }
-  padding-bottom: $padding-base-vertical;
-  &:empty {padding: 0;}
-
-  ul.list-unstyled {
-    margin: 0;
-  }
-}
-
 /* ------ Admin ------ */
 .form-admin-maintenance .form-control {
   display: inline-block;

themes/bootstrap3/templates/Auth/AbstractBase/newpassword.phtml

@@ -8,7 +8,6 @@
   <div class="form-group">
     <label class="control-label" for="oldpwd"><?=$this->transEsc('old_password') ?>:</label>
     <input type="password" name="oldpwd" id="oldpwd" class="form-control" autocomplete="current-password">
-    <div class="help-block with-errors"></div>
   </div>
 <?php endif; ?>
 <div class="form-group">
@@ -21,10 +20,8 @@
   <?php if ($hint = $this->passwordPolicy['hint'] ?? null): ?>
     <div class="help-block"><?=$this->transEsc($hint) ?></div>
   <?php endif; ?>
-  <div class="help-block with-errors"></div>
 </div>
 <div class="form-group">
   <label class="control-label" for="password2"><?=$this->transEsc('confirm_new_password') ?>:</label>
   <input type="password" name="password2" id="password2" class="form-control" required aria-required="true" data-match="#password" data-match-error="<?=$this->transEscAttr('Passwords do not match')?>" autocomplete="new-password">
-  <div class="help-block with-errors"></div>
 </div>

themes/bootstrap3/templates/Auth/Database/create.phtml

@@ -9,7 +9,6 @@
 <div class="form-group">
   <label class="control-label" for="account_email"><?=$this->transEsc('Email Address')?>:</label>
   <input id="account_email" type="email" name="email" required aria-required="true" value="<?=$this->escapeHtmlAttr($this->request->get('email'))?>" class="form-control" autocomplete="email">
-  <div class="help-block with-errors"></div>
 </div>
 <div class="form-group">
   <label class="control-label" for="account_username"><?=$this->transEsc('Desired Username')?>:</label>
@@ -21,7 +20,6 @@
   <?php if ($this->usernamePolicy['hint']): ?>
     <div class="help-block"><?=$this->transEsc($this->usernamePolicy['hint']) ?></div>
   <?php endif; ?>
-  <div class="help-block with-errors"></div>
 </div>
 <div class="form-group">
   <label class="control-label" for="account_password"><?=$this->transEsc('Password')?>:</label>
@@ -33,10 +31,8 @@
   <?php if ($this->passwordPolicy['hint']): ?>
     <div class="help-block"><?=$this->transEsc($this->passwordPolicy['hint']) ?></div>
   <?php endif; ?>
-  <div class="help-block with-errors"></div>
 </div>
 <div class="form-group">
   <label class="control-label" for="account_password2"><?=$this->transEsc('Password Again')?>:</label>
   <input id="account_password2" type="password" name="password2" required class="form-control" data-match="#account_password" data-match-error="<?=$this->transEscAttr('Passwords do not match')?>" autocomplete="new-password">
-  <div class="help-block with-errors"></div>
 </div>

themes/bootstrap3/templates/myresearch/account.phtml

@@ -8,7 +8,7 @@
 <h2><?=$this->transEsc('User Account')?></h2>
 <?=$this->flashmessages()?>
 
-<form method="post" name="accountForm" id="accountForm" class="form-user-create" data-toggle="validator">
+<form method="post" name="accountForm" id="accountForm" class="form-user-create">
   <?=$this->auth()->getCreateFields()?>
   <?=$this->captcha()->html($this->useCaptcha) ?>
   <div class="form-group">

themes/bootstrap3/templates/myresearch/changeemail.phtml

@@ -14,7 +14,7 @@
   <?php if (!$this->auth()->getManager()->supportsEmailChange($this->auth_method)): ?>
     <div class="error"><?=$this->transEsc('change_email_disabled') ?></div>
   <?php else: ?>
-    <form id="newemail" class="form-new-email" action="<?=$this->url('myresearch-changeemail') ?>" method="post" data-toggle="validator">
+    <form id="newemail" class="form-new-email" action="<?=$this->url('myresearch-changeemail') ?>" method="post">
       <input type="hidden" value="<?=$this->escapeHtmlAttr($this->auth()->getManager()->getCsrfHash())?>" name="csrf">
       <div class="form-group">
         <label class="control-label"><?=$this->transEsc('Email Address') ?>:</label>

themes/bootstrap3/templates/myresearch/newpassword.phtml

@@ -18,7 +18,7 @@
 <?php elseif (!isset($this->hash)): ?>
   <div class="error"><?=$this->transEsc('recovery_user_not_found') ?></div>
 <?php else: ?>
-  <form id="newpassword" class="form-new-password" action="<?=$this->url('myresearch-newpassword') ?>" method="post" data-toggle="validator">
+  <form id="newpassword" class="form-new-password" action="<?=$this->url('myresearch-newpassword') ?>" method="post">
     <input type="hidden" value="<?=$this->escapeHtmlAttr($this->auth()->getManager()->getCsrfHash())?>" name="csrf">
     <input type="hidden" value="<?=$this->escapeHtmlAttr($this->hash) ?>" name="hash">
     <input type="hidden" value="<?=$this->escapeHtmlAttr($this->username) ?>" name="username">

themes/bootstrap3/templates/record/sms.phtml

@@ -8,23 +8,15 @@
     . '<li>' . $this->recordLinker()->getBreadcrumbHtml($this->driver) . '</li> '
     . '<li class="active">' . $this->transEsc('Text this') . '</li>';
 ?>
-<?php $validatorCallback = <<<JS
-    function phoneNumberValidation() {
-      return phoneNumberFormHandler('sms_to', '{$this->validation}');
-    }
-    JS;
-?>
-<?=$this->inlineScript(\Laminas\View\Helper\HeadScript::SCRIPT, $validatorCallback, 'SET'); ?>
 <h2><?=$this->transEsc('Text this') ?>: <span class="title-in-heading"><?=$this->escapeHtml($this->driver->getBreadcrumb())?></span></h2>
-<form method="post" name="smsRecord" class="form-record-sms"<?php if (isset($this->validation) && !empty($this->validation)):?> data-lightbox-onsubmit="phoneNumberValidation"<?php endif; ?>>
+<form method="post" name="smsRecord" class="form-record-sms">
   <?=$this->flashmessages()?>
   <input type="hidden" name="id" value="<?=$this->escapeHtmlAttr($this->driver->getUniqueId())?>">
   <input type="hidden" name="source" value="<?=$this->escapeHtmlAttr($this->driver->getSourceIdentifier())?>">
   <input type="hidden" value="<?=$this->escapeHtmlAttr($this->auth()->getManager()->getCsrfHash())?>" name="csrf">
   <div class="form-group">
     <label class="control-label" for="sms_to"><?=$this->transEsc('Number')?>:</label>
-    <input id="sms_to" type="tel" name="to" value="<?=$this->escapeHtmlAttr($this->to)?>" placeholder="<?=$this->transEscAttr('sms_phone_number')?>" class="form-control">
-    <div class="help-block with-errors"></div>
+    <input id="sms_to" type="tel" name="to" value="<?=$this->escapeHtmlAttr($this->to)?>" placeholder="<?=$this->transEscAttr('sms_phone_number')?>" class="form-control" required<?=!empty($this->validation) ? ' data-validator-region="' . $this->escapeHtmlAttr($this->validation) . '"' : ''?>>
   </div>
   <?php if (is_array($this->carriers) && count($this->carriers) > 1): ?>
     <div class="form-group">