Bug-Fix for subforms / repeatable fields by Lab5-Switzerland · Pull Request #24832 · joomla/joomla-cms

Lab5-Switzerland · 2019-05-07T15:22:51Z

JS Form Validation for Subforms/Repeatable Fields Bug IE - Here is the Solution

What needs to be fixed

In IE(11), Fields of a Subform or Repeatable Form dont validate - Seemingly.
Truth is : IE tries to validate the invisible Subform-repeatable part of the subform - which ofc never contains values by its very nature - thus always producing errors

Why this should be fixed

Because Client side form validation is essential. The bug described just exists, because repeatable fields are relatively new and this issue hasn't been addressed yet.
Also the soloution is beautifully simple and implemented in a minute.

How would you fix it

Add the suggested line ( see below ) in to the File
/media/system/js/validate-uncompressed.js , circa line 129, right after
if(jQuery(fields[i]).hasClass('novalidate')) { continue; }
ADD the following line / code :
if(jQuery(fields[i]).parents().is("template") ) { continue; }

Explanation :
If a field is directly of indirectly encapsulated into a tag, it is part of the subform's/repeatable-field's template (/reserve) part, BUT NOT the actual form in use - and thus is NOT to be treated as the part of the form in actual use. Thus, if the validator sees the a given field has as direct or indirect/farther parent a element, the validator knows that this field is not part of the form and will skipp validation on this field. Viola.
Simple and beautiful solution.
( Reation of the validator is analogue to the line before, where it makes the validator skipp the field, it it sees a "novalidate" class attached ).

Ofc, the soloution must be transferred into the compressed version ( validate.js ) too.

Side Effects expected

None.

Pull Request for Issue #24829

Summary of Changes

3 Lines of Code

Testing Instructions

Create any Subform. Some of the Fields in the Subform need to have validation required.
( e.g. add a field field of type="email", required="true", validate="email" )
Test in IE(11) by filling the form and trying to submit the form.
You'll get a form error, althou you have entered all fields correctly.

Expected result

Expected result in form with unfixed file (validate.js) would be, that you can simply submit the form propperly.

Actual result

Actual result in form with unfixed file (validate.js) is, that you get error messages, althou you seemingly entered everything correctly.

Documentation Changes Required

JS Form Validation for Subforms/Repeatable Fields Bug IE - Here is the Solution ### What needs to be fixed In IE(11), Fields of a Subform or Repeatable Form dont validate - Seemingly. Truth is : IE tries to validate the invisible Subform-repeatable<template> part of the subform - which ofc never contains values by its very nature - thus always producing errors ### Why this should be fixed Because Client side form validation is essential. The bug described just exists, because repeatable fields are relatively new and this issue hasn't been addressed yet. Also the soloution is beautifully simple and implemented in a minute. ### How would you fix it Add the suggested line ( see below ) in to the File **/media/system/js/validate-uncompressed.js , circa line 129**, right after if(jQuery(fields[i]).hasClass('novalidate')) { continue; } **ADD** the following line / code : **if(jQuery(fields[i]).parents().is("template") ) { continue; }** Explanation : If a field is directly of indirectly encapsulated into a <template> tag, it is part of the subform's/repeatable-field's template (/reserve) part, BUT NOT the actual form in use - and thus is NOT to be treated as the part of the form in actual use. Thus, if the validator sees the a given field has as direct or indirect/farther parent a <template> element, the validator knows that this field is not part of the form and will skipp validation on this field. Viola. Simple and beautiful solution. ( Reation of the validator is analogue to the line before, where it makes the validator skipp the field, it it sees a "novalidate" class attached ). Ofc, the soloution must be transferred into the compressed version ( validate.js ) too. ### Side Effects expected None.

Fedik · 2019-05-07T16:02:33Z

2 note:

it only affect an old EI, so I would not run this hack for every browser, but make some extra check if (IE and template), because of performance
instead of jQuery(fields[i]).parents().is("template") use jQuery(fields[i]).closest("template")

Lab5-Switzerland · 2019-05-07T23:14:39Z

2 note:

* it only affect an old EI, so I would not run this hack for every browser, but make some extra check `if (IE and template)`, because of performance

* instead of `jQuery(fields[i]).parents().is("template")` use `jQuery(fields[i]).closest("template")`

Great ideas !
I already checked ONE of your suggestions ( .closer() ), which i found great - unfortunately that one has a problem (buggy in Joomla!'s jQuery in IE11).

1.a) Are you shure it only affects an old IE ?
2.a) what's the appropriate line for the IE check ? Im not sure about it. If you have it at hand that'd be great.

2.)
jQuery.closest() WOULD been a great idea, BUT : for some strange reason, it skipps the whole form.
In the end, it seems to be a jQuery based bug anyway in conjunction with IE , evidenced by the behaviour :

althou structurally / hirarchically, the element lies above the field and BESIDE some ancestor element, closest() will hit on it in IE11 althou it should definitely not ( as far as my understanding of jQuery.closest() goes - it should only hit on elements in DIRECT ancestorial line, not sibblings of ancestors. ( Correct me if i'm wrong )).

Fedik · 2019-05-11T07:50:23Z

sorry, forget what I wrote :)

it more like a workaround, but not a correct fix,
need to looks at subform script to fix IE compatibility for this case.

Quy · 2019-05-15T19:53:12Z

media/system/js/validate-uncompressed.js

 	 			continue;
 	 		}
+			// If field is part of a subform-repeatable template, it's not supposed to be validated.
+ 	 		if(jQuery(fields[i]).parents().is("template") ) {


Suggested change

if(jQuery(fields[i]).parents().is("template") ) {

if (jQuery(fields[i]).parents().is("template")) {

OK, tidied up - Both lil changes implemented.

Quy · 2019-05-15T19:54:11Z

Please use the following minifier to submit the compressed version.
https://skalman.github.io/UglifyJS-online/

Lab5-Switzerland · 2019-05-16T02:29:53Z

Please use the following minifier to submit the compressed version.
https://skalman.github.io/UglifyJS-online/

Did now. Hope everything went well.

Fedik · 2019-05-18T16:33:44Z

please, better test this two:
#24947 for Joomla! 3
#24944 for Joomla! 4

joomla-cms-bot · 2019-05-18T17:20:43Z

Set to "closed" on behalf of @Quy by The JTracker Application at issues.joomla.org/joomla-cms/24832

Quy · 2019-05-18T17:20:44Z

Closing in favor of #24947.

Thanks.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24832.}

Lab5-Switzerland requested a review from wilsonge as a code owner May 7, 2019 15:22

joomla-cms-bot added the PR-staging label May 7, 2019

Lab5-Switzerland changed the title ~~24829~~ Bug-Fix for subforms / repeatable fields - Fix for Issue #24829 May 7, 2019

ghost mentioned this pull request May 7, 2019

JS Form Validation for Subforms/Repeatable Fields Bug IE - Here is the Solution #24829

Closed

ghost changed the title ~~Bug-Fix for subforms / repeatable fields - Fix for Issue #24829~~ Bug-Fix for subforms / repeatable fields May 7, 2019

Quy reviewed May 15, 2019

View reviewed changes

Lab5-Switzerland added 3 commits May 16, 2019 04:18

Update validate-uncompressed.js

79d88f7

Update validate-uncompressed.js

9bd2780

Update validate.js

70a6859

Quy mentioned this pull request May 18, 2019

Remove required field option on config_domain.xml #22880

Closed

This was referenced May 18, 2019

[4.0] Move a subform template out of the form scope, for IE compatibility. #24944

Closed

Move a subform template out of the form scope, for IE compatibility #24947

Merged

joomla-cms-bot closed this May 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bug-Fix for subforms / repeatable fields#24832

Bug-Fix for subforms / repeatable fields#24832
Lab5-Switzerland wants to merge 4 commits intojoomla:stagingfrom
Lab5-Switzerland:staging

Lab5-Switzerland commented May 7, 2019

Uh oh!

Fedik commented May 7, 2019

Uh oh!

Lab5-Switzerland commented May 7, 2019 •

edited

Loading

Uh oh!

Fedik commented May 11, 2019

Uh oh!

Quy May 15, 2019

Uh oh!

Lab5-Switzerland May 16, 2019

Uh oh!

Quy commented May 15, 2019

Uh oh!

Lab5-Switzerland commented May 16, 2019

Uh oh!

Fedik commented May 18, 2019

Uh oh!

joomla-cms-bot commented May 18, 2019

Uh oh!

Quy commented May 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	if(jQuery(fields[i]).parents().is("template") ) {
	if (jQuery(fields[i]).parents().is("template")) {

Uh oh!

Conversation

Lab5-Switzerland commented May 7, 2019

What needs to be fixed

Why this should be fixed

How would you fix it

Side Effects expected

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

Uh oh!

Fedik commented May 7, 2019

Uh oh!

Lab5-Switzerland commented May 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fedik commented May 11, 2019

Uh oh!

Quy May 15, 2019

Choose a reason for hiding this comment

Uh oh!

Lab5-Switzerland May 16, 2019

Choose a reason for hiding this comment

Uh oh!

Quy commented May 15, 2019

Uh oh!

Lab5-Switzerland commented May 16, 2019

Uh oh!

Fedik commented May 18, 2019

Uh oh!

joomla-cms-bot commented May 18, 2019

Uh oh!

Quy commented May 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Lab5-Switzerland commented May 7, 2019 •

edited

Loading