looks like not bad idea

@Fedik, thanks for your comments ,

About

jQuery(form).find('label')

Actually if i had thought of it and i wish it could be like that , but it can not be ...
According to HTML specifications ... fields (and their labels) can be placed outside:

<form> ... </form>

• and the way it is done is by using the form attribute
  

<input ...  form="form-TAG-ID" />

... which gives me the opportunity to say that current isValid() implementation needs fixing this !
it needs to use

• form.elements + fieldsets to be compatible with HTML5 specification

About not using,:
$el.data('label') === false;

• used it like === undefined to be backwards compatible

... and because

• === false means field does not have a label
• === undefined means the field has not been examined yet (e.g. 1st validation or newly injected element)

about
parentTagName = $parentElem.get(0).tagName.toLowerCase();

• we already use it in other places
• it computation effect is minimal and used only for some fields only
• not all document types guarantee it to be uppercase
• in future the standards maybe different

about no need to use form.elements

• i remember needing this to properly work with

<fieldsets class="radio" ...>
<fieldsets class="checkbox"  ...>

but i will check, i am not sure if it is needed, but anyway it will not provide any speed benefit (i think)

• also form.elements contains for sure the elements *that are outside the *form:

<form>

According to HTML specifications ... fields and their labels can be placed outside

maybe it could be, but it never happens in Joomla world, I think

$el.data('label') === false;
keep existing behaviour and avoid breaking some 3rd party code

I do not see b/c problem here.
Before use $el.data('label').length they already should check that $el.data('label') exists, other way the code $el.data('label').length !== 0 will fail, when in some contingency the label will not exist (can be removed by other js or so)
JavaScript is dynamic thing

parentTagName = $parentElem.get(0).tagName.toLowerCase();
not all document types guarantee it to be uppercase

one more note, you need to use nodeName instead tagName, then all be fine

-1- about ... searching label only inside < form > using: jQuery(form).find('label')

• it is better to be compliant (if / when possible while having little performance gain to break standards)
• but also current Joomla code checks all the DOM, so ... we would change old behaviour, if we only searched inside the FORM block

so better let it be compliant and B/C too

-2- about using nodeName instead of tagName, i agree with you that it is better

• just for the purpose of checking if the node is a LABEL , it makes no difference in other case yes it would make difference e.g. if checking if element is a text node
• nodeName also supports these nodes: attribute nodes, text nodes, comment nodes

i will replace it (still current code works)

-3-
about checking
$label = $elem.data('label');
and checking .length

• it was like this before ... (current joomla code) always return a jQuery object with $form.find('label[for="' + id + '"]');

Effectively these must and do have the same effect:
Old code:

$label = $form.find('label[for="' + id + '"]');

New code:

if (...needed...) refreshFormLabels(form, form.elements);
$label = $elem.data('label');

both will guarantee a jQuery object
so we do not need to check if it is null / false / undefined
because the new code of findLabel() does the same thing as the previous

• it will return a jQuery object ... and if label was not found then it returns zero length one

@Fedik

I just saw that i responed to a previous message in a confusing way
i am sorry, i wrongly said:

... $elem.data('label') ....

• === false means field does not have a label

i meant if findLabel() returns false then label was not found

$elem.data('label') can only be these

• undefined (label not yet searched)
• jQuery Object with non-zero length (aka label was found)
• jQuery Object with zero length (aka label was NOT found)

now 2 things:

-1- about your suggestion to setting $elem.data('label') to false instead instead of empty jQuery object

• there is other code that examines it like in validateForm() of html5fallback.js, and maybe 3rd party code ? (i don't know)

so it is like the above for B/C, thus we much match the behaviour of what the selector would do:
$form.find('label[for="' + id + '"]');
it would return a zero length jQuery object if label was not found

-2- but your suggestion to check for non-null / not undefined value after refreshFormLabels() makes sense in ... case that refreshFormLabels() is bogus, or in case it was mis-used,

• i will add check for it too,

but still it should never happen with current code

well, maybe you are right about use empty jQuery object in $elem.data('label'),
but still, before check $elem.data('label').length there always need to check that $elem.data('label') actually returns some value, cause the label easily can be removed by external script

yes, i will add check for it

about removing label by external script and letting the field in place
you are right

• but it is usually the opposite, leave the label in place and replace the field, OR ... replace both, in these cases the code will work
• plus current Joomla JS code does not check if the label has been removed !, current Joomla code will use whatever is stored inside .data('label')
• and here we are discussing about if .data('label') is set to some value (removed by 3rd party script ??), and not discussing if the label has been removed, which i thought to add check, but since current Joomla code does not check and because it would code extra, i have not added it

now, i am adding both of your suggestions

• for checking if .data('label') is non-empty
• use nodeName instead of tagName for checking the label

updating html5fallback.js too

@Fedik

i was about to replace tagName with nodeName, and i have just thought that the code may not be needed at all !
(and removing it, would be good for performance too, in case of many label missing e.g. buttons)

if ($label.length <= 0)
{
    var $parentElem = $elem.parent();
  parentTagName = $parentElem.get(0).tagName.toLowerCase();

    if(parentTagName == "label")
    {
        $label = $parentElem;
        $elem.data('label', $label);
    }
}

look at it ... it catches the case of a input inside a label TAG which does not have either of atributes: id="ID-lbl" and for="ID", otherwise it would have been found already by refreshFormLabels()

• i added to validate.js because it exists in html5fallback.js

so because

• there is no such joomla code to create such a thing (there is no such form element)
• and because current JS code of validate.js is not checking it

i think to remove it, not encourage someone writting and using this case and also improving performance in some cases

but it does exist in existing code of html5fallback.js, shall i leave it for html5fallback.js and only remove it from validate.js ?

@Fedik

i have changed the
elem.data('label') to store false if label is missing as you had suggested

• that is how it was before this pull request, but i was trying to make the elem.data('label') to be shared with the findLabel() of html5fallback.js, because for html5fallback.js the return value of findLabel() is an empty jQuery object instead of false,

To summarize new changes / fixes
1. now it is doing both:
-- setting elem.data('label') to false if label is missing
-- elem.data('label') is now properly shared with html5fallback.js, in case that html5fallback.js is re-used in the future (your recently merged PR has suppressed its validateForm())
2. also added a check for the case that an invalid field has no label, current behaviour is not add any message at the top of the page, with this new check if label is missing the invalid field message will be added inline
3. also removed the case of checking input inside label that lucks both id="ID-lbl" and for="ID", no Joomla code is using this case, i had added it because it existed in html5fallback.js

@ggppdk I not sure whether worth to do anything with html5fallback.js
but thanks for your work, I try test more, as soon as find a time

btw, did you already checked about use form.elements vs form?

Hello

fixed the issue with form.elements not being used properly,

i found when bug was introduced,

• it was when mootools code of validation.js was replaced with jQuery code

763c69f

with my last commit the issue of:

• not all form field being included in validation (fields outside < form >)
• and of fields of different forms being included in the validation, that have attribute form="other-form-id" is fixed

@ggppdk good find
also I see that the old code have searched the labels in similar way as you have suggested

if (!(el.labelref)) {
    var labels = $$('label');
    labels.each(function(label){
        if (label.get('for') == el.get('id')) {
            el.labelref = label;
        }
    });
}

but without caching

About old code being similar

• yes similar, in terms of both using a label iterration, but that is where the similarity stops
• the main difference is that my code (both) does and guarantees label iteration to only happens once,

Because

1. during iteration it updates all form fields (and the fieldset class=radio/checkbox) and not only a single field

2. and also for fields without a label, a "false" value is set to indicate no label found and thus avoiding the iteration to be triggered again

• Iteration will never be triggerred again unless the DOM is modified to inject a new form field

That old code was slow
so the existing Joomla code

• replaced it with HTML tag id look-up: ID-lbl

which is fast becaue of the TAG ID look,
but if a field does not have a label with (ID-lbl)

• then the fallback is a full DOM search with the selector:

var $label = $('label[for="'+$elem.attr('id')+'"]');

Which this PR fixes

[EDIT]
for compatibility the create label hash contains all known needed case:
id="ID-lbl"
for="ID"

• in future if we need more cases for the label hash we can add there with minimal cost e.g. marking the label of a fieldgroup ...

I have made some test, and a I am confused
The result not like I have expected.

I have made test in Global Configuration form.

Test the form with 200 of User groups (around 3000 inputs)

Before patch
first validation 143ms

second validation 31ms

After patch
first validation 164ms

second validation 43ms

In result the suggested patch is "slower".

Test the form with 200 of User groups + 10000 inputs (around 13000 inputs)

Here I have added the inputs as in the testing instruction.

Before patch
first validation 875ms

second validation 296ms

After patch
first validation 1010ms

second validation 130ms

In result the suggested patch have won only second "validation".

No idea
Maybe I have missed something.

Hello

the degrade in both cases is small and it is expected
(see explanation below)

• if you have a form that all labels have
  ID-lbl you will see either no benefit or small degrade (as you see above)

• also if you have a form with many "excluded" fields (e.g. ACL for 200 usergroups) again you will see small degrade on 1st validation because , the loop to find loop does not exclude them and makes a single pass for them

in any case the degrade is 10-15%

but for cases that this PR describes

the existing code can be up to 40x slower which huge performance issue for existing code
and that is what this PR fixes

Also at the time i forgot to say

• you see 10%-15% degrade , and i explained why above,

but i forgot to say why your test is different than my initial posting in this PR

your tests did not included validateForm() from html5fallback , right ?

• at the time i tested the validateForm() of html5fallback was still enabled / running

still this PR fixes performance issue with these 2 cases

• many labels missing
• many labels do not have id="ID-lbl"

in all other cases you will see small performance degrade on 1st run, since the validateForm() of ... html5fallback no longer runs

@Fedik

Thanks for the time you took to test this

1 more comment:
every time the cases that this PR fixes, are found by a user

• then you get an performance issue reported here,
• then a PR follows to fix the issue (e.g. exclude fields from the validation loop)

if we would rather do the above, or if we do not care of forms of 3rd party extensions being slow, because they have not "patched" their forms in a similar way, then we may ignore this PR

hm yes, without validateForm(), on latest staging branch
but case when both validateForm() and isValid() executed is a , should be executed only one validation.

Yes i don't say to re-enable validateForm() ,

it only needs to run to do cleanups,

• only cleanup that i have seen is: remove placeholder text in no-supported browsers (IE8)

but anyway placeholder="..." is broken in IE8 (will cause a JS error into html5fallback.js)

so the text never gets added inside the input,

so indeed there is no need to remove it on submit either

@Fedik

you have only made 1 of my 3 tests, there are 2 more test, please try 10,000 inputs with them:

Labels missing

$use_labels = 0;
$add_for = 1;
$add_dash_lbl = 1;

Labels not having id="ID-lbl"

$use_labels = 1;
$add_for = 1;
$add_dash_lbl = 0;

Ok, I have made one more try ????
And in this synthetic test the result more visible

1 From test instruction

$use_labels = 1;
$add_for = 1;
$add_dash_lbl = 1;

Before patch:
first validation 844ms:

second validation 280ms:

After patch:
first validation 762ms:

second validation 328ms:

2 Labels missing

$use_labels = 0;
$add_for = 1;
$add_dash_lbl = 1;

Before patch:
first validation 9s:

second validation 245ms:

After patch:
first validation 569ms:

second validation 299ms:

3 Labels not having id="ID-lbl"

$use_labels = 1;
$add_for = 1;
$add_dash_lbl = 0;

Before patch:
first validation 27s:

second validation 288ms:

After patch:
first validation 771ms:

second validation 341ms:

though it is synthetic, I guess this can be count as success test ????

I have tested this item ✅ successfully on cef7b5d

This PR has received new commits.

CC: @Fedik

This PR has received new commits.

CC: @Fedik

This PR has received new commits.

CC: @Fedik

@Fedik
Thanks for all your comments, i will re-do and split this PR into 3

• label search performance
• fix for not all HTML form fields including in validation
• inline message validation messaging if label is missing

i am already using these in custom software