FIX: premature browser validation notice https://github.com/Crocoblock/issues-tracker/issues/18733

[ObitoTM]

No description provided.

[github-actions]

🤖 AI PR Review

Risk level: low

Review

Summary

• This PR changes the browser reporting behaviour to avoid showing the browser validation notice prematurely. In BrowserReporting.validateOnChange() the code now calls getErrors() and updates validityState.current / clearReport() instead of calling validate(), which would trigger report() and show validation UI.

Files touched

• assets/src/frontend/main/reporting/BrowserReporting.js
  • Function: BrowserReporting.prototype.validateOnChange
  • Change: replaced this.validate().then(...).catch(...) with this.getErrors().then(errors => { this.validityState.current = !Boolean(errors.length); if (!errors.length) { this.clearReport(); } }).catch(...)
• assets/build/frontend/main.js and assets/build/frontend/main.asset.php
  • Built assets updated (version bump). Make sure built file is in sync with source.

Review / Impact

• Correctness: The change is focused and correct for the reported issue. Calling getErrors() avoids call paths that trigger reporting (and therefore browser validation notices) during on-change checks. Clearing report when there are no errors restores previous cleared state.

• Backward compatibility: This is non-breaking for public APIs. It only changes client-side validation behaviour to avoid unnecessary browser notice – existing flows that depend on report() being invoked on change may see different timing (reporting is intentionally suppressed here). Any code that relied on validate() being fired synchronously from validateOnChange would no longer observe the report side-effects; but that was the reported bug so this change is expected.

• Security: No security concerns (no server-side or capability changes). All data handling changes are client-side only.

• Performance: getErrors() may still execute the same async validation promises as validate(); this is equivalent in cost to the previous call to validate() which also relied on the same underlying promise pipeline. No extra heavy operations introduced.

• Multisite / integrations / large forms: Nothing in this change touches server processing or form schema mapping. However, because validation timing changed, please smoke test multi-step forms, repeaters, and integrations (Elementor/JetEngine/booking forms) to ensure no regressions in UX (focus/scroll, message insertion, submit gating).

Potential concerns and recommendations

1. Ensure built assets match source: The PR includes changes to the built asset (assets/build/frontend/main.js) and main.asset.php version bump. Verify the build step was run from the updated source and that source maps (if used) align.

2. Behavioural side-effects: Previously validateOnChange invoked validate(), which could call report() when errors existed. Now report() is not invoked on change; instead validityState.current is updated and clearReport() is called only when no errors. This reduces visible browser validation messages but also changes when report() side-effects occur. Confirm there are no other flows relying on a report() call during on-change.

3. Testing: Add manual/automated checks for:

   • Single-field validation and errors (required, pattern, custom async restrictions)
   • Multi-step forms and repeaters (ensure auto-scroll/focus behaviour still triggers on invalid fields on submit)
   • File inputs, selects, range, custom elements
   • Elementor/booking/JetEngine contexts where forms are initialized by JS plugin hooks
   • Cross-browser testing (Safari, Chrome, Firefox) since browser validation presentation differs.

4. Optional: Add a short inline comment in BrowserReporting.js explaining why getErrors() is used instead of validate() (to avoid showing built-in browser validation tooltip prematurely) so future maintainers understand the intent.

Testing coverage

• There are no automated tests included in the PR. Given this is a UI/UX change with low risk, add at least one integration test or QA checklist to cover the scenarios above, especially to avoid regressions in submit-time validation and auto-scroll behaviour.

Overall

• The change is small, targeted and addresses the reported issue. I recommend merging after the author confirms built assets were generated from the modified source and after a quick manual QA pass (multi-step/repeater/Elementor and cross-browser), and optionally adding a comment in source to explain the rationale.

Suggested changelog entry

- FIX: prevent premature browser validation notice by adjusting frontend reporting logic (validateOnChange) in form frontend validation