JavaScript validation performance (2)

[Fedik]

This pull speed up the form validation by ignoring fields: rules, filters, assigned as they do not need to be validated anyway.

Results on default test installation:
Before (in edit module form):

Validate take 4ms

After (in edit module form):

Validate take 2ms

I count time between push Save button (validation start) and form submit (validation finished).

In result (on default test installation) I get 2 times faster validation, but main target is users who have hundreds of User groups.

Test
Create 200 user groups and compare performance on save the Global Configuration and any Module form.
Also test other forms (on frontend also), whether they still work as expected

UPD: Graph with the final result in that comment #7460 (comment)

[Fedik]

@erimkus you still have site with 250 User groups? can you check whether current fix helps you? 😄

[erimkus]

No sir, back down to 6 groups now. Thank you though.
On Jul 18, 2015 7:27 AM, "Fedir Zinchuk" notifications@github.com wrote:

@erimkus https://github.com/erimkus you still have site with 250 User
groups? can you check whether current fix helps you? [image: 😄]

—
Reply to this email directly or view it on GitHub
#7460 (comment).

[Fedik]

some real test result,
test made in the Global configuration with around 400 user groups:
before:

Submit tok 37 sec

after:

Submit tok 24 sec

win around 10 sec (on my PC)
seems there also need some fix for html5fallback 😱

[Fedik]

so, final result,

test made in the Global configuration with around 400 user groups:
before:

Submit take 37 sec

after:

Submit take 280ms (!!!)

win 99.9% 😄

[dgrammatiko]

@test works 👍

[Fedik]

hm, why Travis filed for php 5.6 but for other is fine? 😄

[zero-24]

@Fedik it is:

FILE: .../joomla-cms/administrator/components/com_config/model/field/filters.php
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 86 | WARNING | Line exceeds 150 characters; contains 180 characters
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------

[Fedik]

yes, but I wonder why the test pass for other versions 😉

[dgrammatiko]

@Fedik PHPCS runs only for PHP 5.6...

[Fedik]

ok, thanks for explanation 😉

[dgrammatiko]

https://github.com/joomla/joomla-cms/blob/staging/.travis.yml#L21

[aantic]

I tested this patch and it works. Save and Close instead of 10 seconds before the patch it takes 1 second after the patch has been applied!

---

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

[andrepereiradasilva]

@aantic please mark as tested https://issues.joomla.org/tracker/joomla-cms/7360 (after login the Test button appears). See https://docs.joomla.org/Testing_Joomla!_patches#Recording_test_results

[aantic]

I have tested this item ✅ successfully on b437f5a

I successfully tested this patch. The Save and Close, Save as Copy, Close and Save button response time in Module Manager and Menu Manager is greatly reduced from 6-10 second to under 1 second! This was tested with Joomla CMS having more that 2000 menus and 400-500 modules!

---

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

[brianteeman]

Can you fix the conflicts first please

[joomla-cms-bot]

This PR has received new commits.

CC: @aantic, @AnneKlapwijk, @RemcoJanssen

---

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

[Fedik]

yes, updated, was just a conflict between the minified script version

[brianteeman]

OK thanks. I will make it RTC this evening

[ggppdk]

This patch is nice,

• it does exclude fields that do not need validation
  and this helps

But why do these fields cost a lot anyway during validation ?

• what is special about them that makes them cost so much ?

well it is the label search in both validation for

• isValid() in validate.js
• validateForm() in html5fallback.js
• validate.js
• if the fields does not have a label with id: ID-lbl then it does a full DOM search with selector like:

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

and most of the fields that are excluded by this patch do not have ID-lbl thus a full DOM search done for each of them,

• html5fallback.js , the situation here is even more bad,
• it does not even try to find label via ID-lbl, it immediately tries the full DOM Search
• furthermore the label that is found is not cached in case it needs to be found again (and it is needed twice in html5fallback validateForm, because of "renderErrorMessages")

i have a form about 500 elements and
validateForm (html5fallback.js) takes *2.3 seconds *,

• after converting the label search to make a single pass and store the labels in hash (object) it take 0.13 seconds
• this one-time calculation is best to be done on every validation try, because it is anyway very cheap and also to allow user to do any manipulation of the form until user clicks submit again (after user's first submit failed because of invalid element)

But it is late and i may be writing something stupid

I will confirm everything tomorrow / or on saturday and make a PR , I found the above while looking through validation code to make the PR for the max_input_vars workaround

• and i was sure that label search has been replaced with an one-time calculated hash,

because i remember this discussion before, so i don't understand why that code is there, the way it is now

but again i maybe missing something

[Fedik]

I feel like I need to answer 😃

All what you wrote is correct.
But some more.

With html5fallback.js here was fixed multiple problems:

• First problem, the validation was executed twice: once by validate.js and second by html5fallback.js.
• Another problem was that html5fallback.js run validation even you push Cancel.

From my point of view html5fallback.js it is totally useless thing, no idea why we use it 😄
for placeholder? well, there more lightweight solutions,
for validation? nope, because we use validate.js, that has some additional API which allow to register custom "validators"

i have a form about 500 elements and

You know, with 200 user groups/modules you will have a couple thousands different inputs 😉

[ggppdk]

well both of them, have performance issue with labels

• just with validate.js the problem appears only when you have a form with elements that
• do not have HTML tag id like ID_of_field-lbl
• or do not have a label at all like the close buttons that this PR excluded !

i have made a fix for performance issue with the labels,
and i am testing it

fix also includes check for

• injected elements that means if you try validation and then you inject elements into the form , then you can not rely on the previously calculated hash
  [EDIT] will use jQuery data() to hold the labels, and update these if missing

thus works with both cases of full form validation / single field validation

• isValid() / validateForm() (validate.js/html5fallback.js)
• validate() / validateField() (validate.js/html5fallback.js)

for validate.js it is as you say Joomla allows to override it and this is very convenient for custom components

... about how useful html5fallback.js well now it is supposed to make joomla forms browser agnostic and it is a couple of things more than placeholder support by the browser

• validate.js handles custom validation cases,
• html5fallback handles validation cases that are natively supported by the browser and also things like placeholder, to be honest, i have not studied it that much

[ggppdk]

Regardless of avoiding full DOM search for finding individual labels of fields

• this PR added novalidate class that was needed anyway to avoid redudant work completely !

[ggppdk]

@Fedik

From my point of view html5fallback.js it is totally useless thing, no idea why we use it 😄
for placeholder? well, there more lightweight solutions,

yes you are right HTML5 validation attributes are not used
it only handles ... placeholder and ?

Furthermore validateForm() of html5fallback.js runs after isValid() succeeds
thus adding a lot of delay (due to findLabel) in large forms

I have enabled fixed findLabel performance e.g. 10,000 fields (e.g. like buttons that you excluded)
before fix: 15 seconds after fix 300 milliseconds

also enabled HTML5 validation for modern browsers / IE10+

• for older no error, just HTML5 validation does not run
  

[ggppdk]

@Fedik , @dgt41 , @mbabker

Question about:
validateForm() of html5fallback.js
is it no longer needed ?
because before this PR it was running , with this PR it no longer runs

this PR adds into
Joomla.submitform = function(task, form, validate)
a new line
https://github.com/joomla/joomla-cms/pull/7460/files#diff-c1b1173fc1376e9afd63e8ec801c711aR33

form.setAttribute('novalidate', !validate)

and currently validate parameter is never passed by any form,
thus it always false (more precisely it is undefined, aka !validate sets novalidate to true)

is this what we want ?

[EDIT]
Also the weird thing about html5fallback.js is that

• it tries to add HTML5 validation (e.g. pattern, required, etc) for browser that DO NOT support it
• but for browser that support HTML5 validation it does not trigger it via checkValidity()

[Fedik]

because before this PR it was running

It was running accidentally (because submit event) 😄

and currently validate parameter is never passed by any form, thus it always false ...
is this what we want ?

yeap, it for keep an old behavior, see #6587

In my sites I have made hack by plugin that removes html5fallback.js (and some other) and all works fine, more than year already 😄
But I rarely use frontend forms, so cannot say much about frontend. In most cases there work the browser native validation, and only "server side validation" for old browsers.

[ggppdk]

yes i had seen that PR #6587, and had i understood the reason and what it does

your answer is helpful , thanks !
because i see now

• where and how to place the workaround for max_input_vars,

e.g. i can not be placed inside html5fallback.js, as this may not be loaded or we can even consider it optional

In my sites I have made hack by plugin that removes html5fallback.js (and some other) and all works fine, more than year already

It is best to place the new code inside core.js
(only forms that eventually call Joomla.submitform() (Joomla.pressbutton() calls it) will benefit from the fix, which is ok)

To be more precise the code for:
checking and registering (only once) the extra on-submit handler for compressing the form (max_input_vars workaround),

• this must be just the hidden submit button is clicked, thus ensuring that it will run once after any other handlers has been added (since all of them are registered at load-time but in any case before our handler), the obvious reason for need to run last, is because it tampers with form, by compressing it into a single variable and disabling all the inputs
• then after click() add code to restore form state just in case that this is needed (e.g. if we add AJAX submit feature or form did not submit for any ? reason but ... compression was run, which it should not be possible if preventdefault was called), like the code that removes the extra injected submit button

So i will make

• 1 PR (almost finished / tested it) to update validate.js / html5fallback.js and do / fix:
• performance issue with findLabel() in both validate.js / html5fallback.js
• allow HTML5 validation to be checked by supported browsers like i described above (i know it will not be used but still it is good to have for future)
• fix for not checking form elements that are outside <form> and use the form="form-id" attribute
• 1 PR for core.js and PHP files for the max_input_vars / suhosin limits workaround
• 1 PR (in near future ?) for AJAX form submit (no form reloading)

[ggppdk]

@Fedik

Ok i figured out why HTML5 validation is not running
as said above this PR adds:

form.setAttribute('novalidate', !validate)

• but novalidate attribute is like disabled and required attributes , meaning only its existence / non-existence counts, thus the value that it contains is not relevant

and because novalidate is the HTML5 specification attribute for stopping HTML5 validation

• it is not only suppressing validateForm() of HTML5fallback JS, but it is also suppressing browser's built-in HTML5 validation

i think this is a behaviour change, some forms maybe using HTML5 validation !

here is a more ? proper code :

// Toggle HTML5 validation

if (typeof validate === 'undefined' || validate === null)
    ; // Allow HTML5 validation according to form original code (when not set or null)

else
    !validate ?
        form.setAttribute('novalidate', 'novalidate') :   // Force HTML5 validate OFF
        form.removeAttribute('novalidate') ;              // Force HTML5 validate ON

[Fedik]

i think this is a behaviour change, some forms maybe using HTML5 validation !

via Joomla.submitform they may use it after that pull #6587 , before that it was not possible ... but I have huge doubt that there at least 1 use it 😄

[ggppdk]

via Joomla.submitform they may use it after that pull #6587 , before that it was not possible ...
but I have huge doubt that there at least 1 use it

Yes it is exactly , as you say above,

• before that pull request it was not possible
• and doubt too if anyone is using HTML5 validation now
• and this PR restored the previous behaviour

but still the question is , do we want to prevent HTML5 validation from running ?

[Fedik]

but still the question is , do we want to prevent HTML5 validation from running ?

I think for Joomla! 3.x.x we do not have much choose. As we need to use validate.js for keep old behavior, because some extension could use it, with custom validation handlers (similar to #9997)

[ggppdk]

I did not say to change anything in Joomla validation code (validate.js) it does not need to change

• except of course of fixing the really bad performance of findLabel()

i only said why stop HTML5 browser's validation ?
because HTML5 browser's validation, will only be triggered if it exists inside the form's HTML

• if it does not exist then it does not run, now if someone adds the HTML5 code we suppress it

If we want to remove html5fallback.js, that is another topic but it is not related to allowing HTML5 validation to be runned by the browser

anyway i will make a PR soon

[ggppdk]

There is only 1 problem with allowing HTML5 validation to run,

• it will run even if submitform was called is via a "cancel" button, but we could set 'novalidate' form attribut if task is:
  '*.cancel | cancel'

[EDIT] we can stop HTML5 validation:

var isCancel = ....;
if (isCancel) form.setAttribute('novalidate', 'novalidate');

[Fedik]

yes, and it already possible, developer can make his own Joomla.submitbutton method that should handle it. Example:

// Toolbar
Joomla.submitbutton('save'); // Save button
Joomla.submitbutton('cancel'); // Cancel button

// Extension specific submit method 
Joomla.submitbutton = function(task){
  var form = document.getElementById('specific-form-id'), 
      validate = task !== 'cancel';

  Joomla.submitform(task, form, validate);  
}

[ggppdk]

Since this was merged into staging, many people are now using it

• just now i have re-tested some forms

What do you mean with "System J3.5.1" ?
Do you mean that you took only the files of this PR copied then into the web-site ?

It is possible to test a PR (before it is merged)

• only against the branch that it was made

because every PR takes for granted that you have all other files of the branch

In this case it was merged into the "staging" branch,
so to test it now, it is better to

• make a copy of your website
• upgrade this new copy, with all files of the staging branch (extract into it),
• then in backend go to : Extensions / Database / click "Fix"
• then test the forms