? ? Success

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
18 Jul 2015

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):
screen 2015-07-18 14 18 27 1064x359
Validate take 4ms

After (in edit module form):
screen 2015-07-18 14 18 46 1071x358
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)

avatar Fedik Fedik - open - 18 Jul 2015
avatar Fedik Fedik - change - 18 Jul 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Jul 2015
Labels Added: ?
avatar Fedik
Fedik - comment - 18 Jul 2015

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

avatar erimkus
erimkus - comment - 18 Jul 2015

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: :smile:]


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

avatar Fedik
Fedik - comment - 19 Jul 2015

some real test result,
test made in the Global configuration with around 400 user groups:
before:
screen 2015-07-19 11 05 36 1282x372
Submit tok 37 sec

after:
screen 2015-07-19 11 06 21 1341x361
Submit tok 24 sec

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

avatar Fedik Fedik - change - 19 Jul 2015
Title
Validation.js performance (2) + IE8 fix
Validation.js performance (2) [WIP]
avatar Fedik Fedik - change - 19 Jul 2015
Title
Validation.js performance (2) + IE8 fix
Validation.js performance (2) [WIP]
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-07-19 08:25:00
Closed_By Fedik
avatar Fedik Fedik - close - 19 Jul 2015
avatar Fedik Fedik - close - 19 Jul 2015
avatar Fedik Fedik - change - 19 Jul 2015
Status Closed New
Closed_Date 2015-07-19 08:25:00
Closed_By Fedik
avatar Fedik Fedik - reopen - 19 Jul 2015
avatar Fedik Fedik - reopen - 19 Jul 2015
avatar Fedik
Fedik - comment - 19 Jul 2015

so, final result,

test made in the Global configuration with around 400 user groups:
before:
screen 2015-07-19 11 05 36 1282x372
Submit take 37 sec

after:
screen 2015-07-19 12 13 51 1114x322
Submit take 280ms (!!!)

win 99.9% :smile:

avatar Fedik Fedik - change - 19 Jul 2015
Title
Validation.js performance (2) [WIP]
JavaScript validation performance (2)
avatar brianteeman brianteeman - change - 19 Jul 2015
Title
JavaScript validation performance (2)
JavaScript validation performance (2)
avatar brianteeman brianteeman - change - 19 Jul 2015
Category JavaScript
avatar dgt41
dgt41 - comment - 26 Jul 2015

@test works ????

avatar Fedik
Fedik - comment - 24 Nov 2015

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

avatar zero-24
zero-24 - comment - 24 Nov 2015

@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
--------------------------------------------------------------------------------
avatar Fedik
Fedik - comment - 24 Nov 2015

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

avatar dgt41
dgt41 - comment - 24 Nov 2015

@Fedik PHPCS runs only for PHP 5.6...

avatar Fedik
Fedik - comment - 24 Nov 2015

ok, thanks for explanation :wink:

43c09d8 24 Nov 2015 avatar Fedik C.S.
avatar aantic
aantic - comment - 11 Jan 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 11 Jan 2016
avatar aantic aantic - test_item - 11 Jan 2016 - Tested successfully
avatar aantic
aantic - comment - 11 Jan 2016

I have tested this item :white_check_mark: 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.

avatar infograf768
infograf768 - comment - 14 Jan 2016

There are conflicts to be solved (on core.js) to test on staging.

avatar aantic
aantic - comment - 14 Jan 2016

I am not sure I understand. Which conflicts need to be solved? Does joomla core files create conflicts and need to be solved?

avatar infograf768
infograf768 - comment - 14 Jan 2016

This concerns @Fedik
He has to solve the conflicts with existing staging branch as this patch has been done a while ago.

avatar aantic
aantic - comment - 14 Jan 2016

OK thanks. The only thing I am sure is that I was unable to work normally
with Save and Close, Save as Copy, Save, and Close buttons (all took more
that 10 seconds to complete actions) until I applied the patch. Now
everything works as a charm as I mentioned.
I plan on having this patch applied until it gets part of Joomla update!

avatar joomla-cms-bot
joomla-cms-bot - comment - 14 Jan 2016

This PR has received new commits.

CC: @aantic


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

avatar Fedik
Fedik - comment - 14 Jan 2016

it just not possible to track the minified version :smile:
updated

avatar aantic
aantic - comment - 14 Jan 2016

When you updated a patch do I need to reapply it within my com_patchtester
component?
I'm sorry if my question is secular but does your patch has any influence
on front end Joomla performance as well?

avatar Fedik
Fedik - comment - 14 Jan 2016

no need reapply, in current case

... has any influence on front end Joomla performance as well?

it mostly for backend performance problem, in fronend you will not see big difference as there the forms is "smaller" :smile:

avatar aantic
aantic - comment - 19 Jan 2016

OK, sorry for the late response. Your patch doesn't influence front end performance.
Back end performance is great as I said, I almost completed the website containing 2000 menus as many articles, and around 400 modules.
I think this is the great website to really test Joomla performance.
I have Joomla and all Extensions up to date, Cache enabled (conservative), GZIP
compression enabled, JCH Optimize plugin enabled.
Also I followed all the rules from here:
https://www.siteground.com/tutorials/joomla/joomla-speed.htm
Unfortunately I still have problems with slow load in front end which concerns me a lot.
This is the test result - http://www.webpagetest.org/result/160119_5H_J4P/
Do you have any idea how I can improve the speed? Thanks in advance!

avatar Fedik
Fedik - comment - 19 Jan 2016

@aantic the patch for the editing form, not for the whole site :wink:

avatar brianteeman
brianteeman - comment - 19 Jan 2016

This isnt the place for specific site support (thats the forum) but if you
look at https://gtmetrix.com/reports/dev.olympic.rs/omqnO7q4 you will see
some really easy gains you can make with your images

On 19 January 2016 at 10:04, Fedir Zinchuk notifications@github.com wrote:

@aantic https://github.com/aantic the patch for the editing form, not
for the whole site [image: :wink:]


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

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar aantic
aantic - comment - 19 Jan 2016

I understand, thanks!

avatar aantic
aantic - comment - 19 Jan 2016

Whit regards to the patch, as Fedik said there must be 2 testers who successfully tested the patch in order for the patch to be included in next Joomla update. If this is true I would start creating a large Joomla website by copying menu items, articles and modules and see if I experience slow down on validation. I would set up the website on my colleague's server so he can test it as well and submit his comment and thus be the second one who successfully tested the patch. Am I on the right track here?

avatar aantic
aantic - comment - 27 Jan 2016

I am a little bit concerned that com_patchtester cannot find the issue number 7460 by typing id:7460 in the filter field?
The only way I can find it is by start typing valid....(because the issue is related to validation).
Do you know why is that?
filter field 1
filter field 2

avatar brianteeman
brianteeman - comment - 27 Jan 2016

you dont use the id in the search with com_patchtester
just type the number

On 27 January 2016 at 16:13, aantic notifications@github.com wrote:

I am a little bit concerned that com_patchtester cannot find the issue
number 7460 by typing id:7460 in the filter field?
The only way I can find it is by start typing valid....(because the issue
is related to validation).
Do you know why is that?
[image: filter field 1]
https://cloud.githubusercontent.com/assets/8791467/12619654/3309c682-c519-11e5-9680-ef3d89486610.jpg
[image: filter field 2]
https://cloud.githubusercontent.com/assets/8791467/12619655/3319b542-c519-11e5-983b-c3bea80b440e.jpg


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

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar aantic
aantic - comment - 27 Jan 2016

It doesn't change
filter field 3
the situation :(

avatar mbabker
mbabker - comment - 27 Jan 2016

@brianteeman That change was made after the last tag. id:7460 should work right. My suggestion: if it's constantly reproducible, file a bug report at https://github.com/joomla-extensions/patchtester and include the SQL queries pulling data for the patch tester tables in the report (you'll need to turn on debug mode for this).

avatar brianteeman
brianteeman - comment - 27 Jan 2016

@mbabker i must still be using an old version - sorry

On 27 January 2016 at 16:18, Michael Babker notifications@github.com
wrote:

@brianteeman https://github.com/brianteeman That change was made after
the last tag. id:7460 should work right. My suggestion: if it's
constantly reproducible, file a bug report at
https://github.com/joomla-extensions/patchtester and include the SQL
queries pulling data for the patch tester tables in the report (you'll need
to turn on debug mode for this).


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

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar joomla-cms-bot
joomla-cms-bot - comment - 27 Jan 2016

This PR has received new commits.

CC: @aantic


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

avatar AnneKlapwijk AnneKlapwijk - test_item - 15 Apr 2016 - Tested successfully
avatar AnneKlapwijk
AnneKlapwijk - comment - 15 Apr 2016

I have tested this item :white_check_mark: successfully on 88c4499

Tested successfully. Without the patch it takes a few seconds before the form (global configuration) is submitted.
After the patch, it takes only a small second!

First, I created an SQL script to create 199 new user groups (see script below).
Then I tested the global configuration form before applying the patch, and after.

Before
Joomla.submitbutton Total Time 16.61 s
Joomla.submitbutton Total Time 15.80 s
Joomla.submitbutton Total Time 15.21 s

After
Joomla.submitbutton Total Time 437.26 ms
Joomla.submitbutton Total Time 449.90 ms
Joomla.submitbutton Total Time 489.66 ms

Screenshot before:
https://drive.google.com/open?id=0By1yAXmUY6tiM3NqQUI0TC1NRjg

Screenshot after:
https://drive.google.com/open?id=0By1yAXmUY6tiQUFaQ1F0OGY1X1U

INSERT INTO `database_name`.`prefix_usergroups` (`id`, `parent_id`, `lft`, `rgt`, `title`) VALUES
(NULL, '6', '5', '6', ' Administrator 1  '),
(NULL, '6', '5', '6', ' Administrator 2  '),
(NULL, '6', '5', '6', ' Administrator 3  '),
(NULL, '6', '5', '6', ' Administrator 4  '),
(NULL, '6', '5', '6', ' Administrator 5  '),
(NULL, '6', '5', '6', ' Administrator 6  '),
(NULL, '6', '5', '6', ' Administrator 7  '),
(NULL, '6', '5', '6', ' Administrator 8  '),
(NULL, '6', '5', '6', ' Administrator 9  '),
(NULL, '6', '5', '6', ' Administrator 10     '),
(NULL, '6', '5', '6', ' Administrator 11     '),
(NULL, '6', '5', '6', ' Administrator 12     '),
(NULL, '6', '5', '6', ' Administrator 13     '),
(NULL, '6', '5', '6', ' Administrator 14     '),
(NULL, '6', '5', '6', ' Administrator 15     '),
(NULL, '6', '5', '6', ' Administrator 16     '),
(NULL, '6', '5', '6', ' Administrator 17     '),
(NULL, '6', '5', '6', ' Administrator 18     '),
(NULL, '6', '5', '6', ' Administrator 19     '),
(NULL, '6', '5', '6', ' Administrator 20     '),
(NULL, '6', '5', '6', ' Administrator 21     '),
(NULL, '6', '5', '6', ' Administrator 22     '),
(NULL, '6', '5', '6', ' Administrator 23     '),
(NULL, '6', '5', '6', ' Administrator 24     '),
(NULL, '6', '5', '6', ' Administrator 25     '),
(NULL, '6', '5', '6', ' Administrator 26     '),
(NULL, '6', '5', '6', ' Administrator 27     '),
(NULL, '6', '5', '6', ' Administrator 28     '),
(NULL, '6', '5', '6', ' Administrator 29     '),
(NULL, '6', '5', '6', ' Administrator 30     '),
(NULL, '6', '5', '6', ' Administrator 31     '),
(NULL, '6', '5', '6', ' Administrator 32     '),
(NULL, '6', '5', '6', ' Administrator 33     '),
(NULL, '6', '5', '6', ' Administrator 34     '),
(NULL, '6', '5', '6', ' Administrator 35     '),
(NULL, '6', '5', '6', ' Administrator 36     '),
(NULL, '6', '5', '6', ' Administrator 37     '),
(NULL, '6', '5', '6', ' Administrator 38     '),
(NULL, '6', '5', '6', ' Administrator 39     '),
(NULL, '6', '5', '6', ' Administrator 40     '),
(NULL, '6', '5', '6', ' Administrator 41     '),
(NULL, '6', '5', '6', ' Administrator 42     '),
(NULL, '6', '5', '6', ' Administrator 43     '),
(NULL, '6', '5', '6', ' Administrator 44     '),
(NULL, '6', '5', '6', ' Administrator 45     '),
(NULL, '6', '5', '6', ' Administrator 46     '),
(NULL, '6', '5', '6', ' Administrator 47     '),
(NULL, '6', '5', '6', ' Administrator 48     '),
(NULL, '6', '5', '6', ' Administrator 49     '),
(NULL, '6', '5', '6', ' Administrator 50     '),
(NULL, '6', '5', '6', ' Administrator 51     '),
(NULL, '6', '5', '6', ' Administrator 52     '),
(NULL, '6', '5', '6', ' Administrator 53     '),
(NULL, '6', '5', '6', ' Administrator 54     '),
(NULL, '6', '5', '6', ' Administrator 55     '),
(NULL, '6', '5', '6', ' Administrator 56     '),
(NULL, '6', '5', '6', ' Administrator 57     '),
(NULL, '6', '5', '6', ' Administrator 58     '),
(NULL, '6', '5', '6', ' Administrator 59     '),
(NULL, '6', '5', '6', ' Administrator 60     '),
(NULL, '6', '5', '6', ' Administrator 61     '),
(NULL, '6', '5', '6', ' Administrator 62     '),
(NULL, '6', '5', '6', ' Administrator 63     '),
(NULL, '6', '5', '6', ' Administrator 64     '),
(NULL, '6', '5', '6', ' Administrator 65     '),
(NULL, '6', '5', '6', ' Administrator 66     '),
(NULL, '6', '5', '6', ' Administrator 67     '),
(NULL, '6', '5', '6', ' Administrator 68     '),
(NULL, '6', '5', '6', ' Administrator 69     '),
(NULL, '6', '5', '6', ' Administrator 70     '),
(NULL, '6', '5', '6', ' Administrator 71     '),
(NULL, '6', '5', '6', ' Administrator 72     '),
(NULL, '6', '5', '6', ' Administrator 73     '),
(NULL, '6', '5', '6', ' Administrator 74     '),
(NULL, '6', '5', '6', ' Administrator 75     '),
(NULL, '6', '5', '6', ' Administrator 76     '),
(NULL, '6', '5', '6', ' Administrator 77     '),
(NULL, '6', '5', '6', ' Administrator 78     '),
(NULL, '6', '5', '6', ' Administrator 79     '),
(NULL, '6', '5', '6', ' Administrator 80     '),
(NULL, '6', '5', '6', ' Administrator 81     '),
(NULL, '6', '5', '6', ' Administrator 82     '),
(NULL, '6', '5', '6', ' Administrator 83     '),
(NULL, '6', '5', '6', ' Administrator 84     '),
(NULL, '6', '5', '6', ' Administrator 85     '),
(NULL, '6', '5', '6', ' Administrator 86     '),
(NULL, '6', '5', '6', ' Administrator 87     '),
(NULL, '6', '5', '6', ' Administrator 88     '),
(NULL, '6', '5', '6', ' Administrator 89     '),
(NULL, '6', '5', '6', ' Administrator 90     '),
(NULL, '6', '5', '6', ' Administrator 91     '),
(NULL, '6', '5', '6', ' Administrator 92     '),
(NULL, '6', '5', '6', ' Administrator 93     '),
(NULL, '6', '5', '6', ' Administrator 94     '),
(NULL, '6', '5', '6', ' Administrator 95     '),
(NULL, '6', '5', '6', ' Administrator 96     '),
(NULL, '6', '5', '6', ' Administrator 97     '),
(NULL, '6', '5', '6', ' Administrator 98     '),
(NULL, '6', '5', '6', ' Administrator 99     '),
(NULL, '6', '5', '6', ' Administrator 100    '),
(NULL, '6', '5', '6', ' Administrator 101    '),
(NULL, '6', '5', '6', ' Administrator 102    '),
(NULL, '6', '5', '6', ' Administrator 103    '),
(NULL, '6', '5', '6', ' Administrator 104    '),
(NULL, '6', '5', '6', ' Administrator 105    '),
(NULL, '6', '5', '6', ' Administrator 106    '),
(NULL, '6', '5', '6', ' Administrator 107    '),
(NULL, '6', '5', '6', ' Administrator 108    '),
(NULL, '6', '5', '6', ' Administrator 109    '),
(NULL, '6', '5', '6', ' Administrator 110    '),
(NULL, '6', '5', '6', ' Administrator 111    '),
(NULL, '6', '5', '6', ' Administrator 112    '),
(NULL, '6', '5', '6', ' Administrator 113    '),
(NULL, '6', '5', '6', ' Administrator 114    '),
(NULL, '6', '5', '6', ' Administrator 115    '),
(NULL, '6', '5', '6', ' Administrator 116    '),
(NULL, '6', '5', '6', ' Administrator 117    '),
(NULL, '6', '5', '6', ' Administrator 118    '),
(NULL, '6', '5', '6', ' Administrator 119    '),
(NULL, '6', '5', '6', ' Administrator 120    '),
(NULL, '6', '5', '6', ' Administrator 121    '),
(NULL, '6', '5', '6', ' Administrator 122    '),
(NULL, '6', '5', '6', ' Administrator 123    '),
(NULL, '6', '5', '6', ' Administrator 124    '),
(NULL, '6', '5', '6', ' Administrator 125    '),
(NULL, '6', '5', '6', ' Administrator 126    '),
(NULL, '6', '5', '6', ' Administrator 127    '),
(NULL, '6', '5', '6', ' Administrator 128    '),
(NULL, '6', '5', '6', ' Administrator 129    '),
(NULL, '6', '5', '6', ' Administrator 130    '),
(NULL, '6', '5', '6', ' Administrator 131    '),
(NULL, '6', '5', '6', ' Administrator 132    '),
(NULL, '6', '5', '6', ' Administrator 133    '),
(NULL, '6', '5', '6', ' Administrator 134    '),
(NULL, '6', '5', '6', ' Administrator 135    '),
(NULL, '6', '5', '6', ' Administrator 136    '),
(NULL, '6', '5', '6', ' Administrator 137    '),
(NULL, '6', '5', '6', ' Administrator 138    '),
(NULL, '6', '5', '6', ' Administrator 139    '),
(NULL, '6', '5', '6', ' Administrator 140    '),
(NULL, '6', '5', '6', ' Administrator 141    '),
(NULL, '6', '5', '6', ' Administrator 142    '),
(NULL, '6', '5', '6', ' Administrator 143    '),
(NULL, '6', '5', '6', ' Administrator 144    '),
(NULL, '6', '5', '6', ' Administrator 145    '),
(NULL, '6', '5', '6', ' Administrator 146    '),
(NULL, '6', '5', '6', ' Administrator 147    '),
(NULL, '6', '5', '6', ' Administrator 148    '),
(NULL, '6', '5', '6', ' Administrator 149    '),
(NULL, '6', '5', '6', ' Administrator 150    '),
(NULL, '6', '5', '6', ' Administrator 151    '),
(NULL, '6', '5', '6', ' Administrator 152    '),
(NULL, '6', '5', '6', ' Administrator 153    '),
(NULL, '6', '5', '6', ' Administrator 154    '),
(NULL, '6', '5', '6', ' Administrator 155    '),
(NULL, '6', '5', '6', ' Administrator 156    '),
(NULL, '6', '5', '6', ' Administrator 157    '),
(NULL, '6', '5', '6', ' Administrator 158    '),
(NULL, '6', '5', '6', ' Administrator 159    '),
(NULL, '6', '5', '6', ' Administrator 160    '),
(NULL, '6', '5', '6', ' Administrator 161    '),
(NULL, '6', '5', '6', ' Administrator 162    '),
(NULL, '6', '5', '6', ' Administrator 163    '),
(NULL, '6', '5', '6', ' Administrator 164    '),
(NULL, '6', '5', '6', ' Administrator 165    '),
(NULL, '6', '5', '6', ' Administrator 166    '),
(NULL, '6', '5', '6', ' Administrator 167    '),
(NULL, '6', '5', '6', ' Administrator 168    '),
(NULL, '6', '5', '6', ' Administrator 169    '),
(NULL, '6', '5', '6', ' Administrator 170    '),
(NULL, '6', '5', '6', ' Administrator 171    '),
(NULL, '6', '5', '6', ' Administrator 172    '),
(NULL, '6', '5', '6', ' Administrator 173    '),
(NULL, '6', '5', '6', ' Administrator 174    '),
(NULL, '6', '5', '6', ' Administrator 175    '),
(NULL, '6', '5', '6', ' Administrator 176    '),
(NULL, '6', '5', '6', ' Administrator 177    '),
(NULL, '6', '5', '6', ' Administrator 178    '),
(NULL, '6', '5', '6', ' Administrator 179    '),
(NULL, '6', '5', '6', ' Administrator 180    '),
(NULL, '6', '5', '6', ' Administrator 181    '),
(NULL, '6', '5', '6', ' Administrator 182    '),
(NULL, '6', '5', '6', ' Administrator 183    '),
(NULL, '6', '5', '6', ' Administrator 184    '),
(NULL, '6', '5', '6', ' Administrator 185    '),
(NULL, '6', '5', '6', ' Administrator 186    '),
(NULL, '6', '5', '6', ' Administrator 187    '),
(NULL, '6', '5', '6', ' Administrator 188    '),
(NULL, '6', '5', '6', ' Administrator 189    '),
(NULL, '6', '5', '6', ' Administrator 190    '),
(NULL, '6', '5', '6', ' Administrator 191    '),
(NULL, '6', '5', '6', ' Administrator 192    '),
(NULL, '6', '5', '6', ' Administrator 193    '),
(NULL, '6', '5', '6', ' Administrator 194    '),
(NULL, '6', '5', '6', ' Administrator 195    '),
(NULL, '6', '5', '6', ' Administrator 196    '),
(NULL, '6', '5', '6', ' Administrator 197    '),
(NULL, '6', '5', '6', ' Administrator 198    '),
(NULL, '6', '5', '6', ' Administrator 199    ')
```<hr /><sub>This comment was created with the <a href="https://github.com/joomla/jissues">J!Tracker Application</a> at <a href="https://issues.joomla.org/tracker/joomla-cms/7460">issues.joomla.org/joomla-cms/7460</a>.</sub>
avatar RemcoJanssen RemcoJanssen - test_item - 15 Apr 2016 - Tested successfully
avatar RemcoJanssen
RemcoJanssen - comment - 15 Apr 2016

I have tested this item :white_check_mark: successfully on 88c4499


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

avatar brianteeman brianteeman - change - 15 Apr 2016
Status New Ready to Commit
avatar brianteeman brianteeman - change - 15 Apr 2016
Milestone Added:
avatar Fedik
Fedik - comment - 21 Apr 2016

as here already more than 2 success test can it have RTC?

avatar joomla-cms-bot joomla-cms-bot - change - 21 Apr 2016
Labels Added: ?
avatar brianteeman
brianteeman - comment - 21 Apr 2016

Can you fix the conflicts first please

avatar joomla-cms-bot
joomla-cms-bot - comment - 21 Apr 2016

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.

avatar Fedik
Fedik - comment - 21 Apr 2016

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

avatar brianteeman
brianteeman - comment - 21 Apr 2016

OK thanks. I will make it RTC this evening

avatar rdeutz rdeutz - change - 21 Apr 2016
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-04-21 17:42:21
Closed_By rdeutz
avatar rdeutz rdeutz - close - 21 Apr 2016
avatar rdeutz rdeutz - merge - 21 Apr 2016
avatar joomla-cms-bot joomla-cms-bot - close - 21 Apr 2016
avatar rdeutz rdeutz - reference | de453c1 - 21 Apr 16
avatar rdeutz rdeutz - merge - 21 Apr 2016
avatar rdeutz rdeutz - close - 21 Apr 2016
avatar brianteeman brianteeman - close - 21 Apr 2016
avatar joomla-cms-bot joomla-cms-bot - change - 21 Apr 2016
Labels Removed: ?
avatar Fedik Fedik - head_ref_deleted - 21 Apr 2016
avatar ggppdk
ggppdk - comment - 22 Apr 2016

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

avatar Fedik
Fedik - comment - 22 Apr 2016

I feel like I need to answer :smiley:

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 :smile:
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 :wink:

avatar ggppdk
ggppdk - comment - 22 Apr 2016

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
avatar ggppdk
ggppdk - comment - 22 Apr 2016

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 !
avatar ggppdk
ggppdk - comment - 23 Apr 2016

@Fedik

From my point of view html5fallback.js it is totally useless thing, no idea why we use it :smile:
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 html5_validation
avatar ggppdk
ggppdk - comment - 24 Apr 2016

@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()
avatar Fedik
Fedik - comment - 24 Apr 2016

because before this PR it was running

It was running accidentally (because submit event) :smile:

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 :smile:
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.

avatar ggppdk
ggppdk - comment - 24 Apr 2016

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)

avatar ggppdk
ggppdk - comment - 24 Apr 2016

@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
avatar Fedik
Fedik - comment - 25 Apr 2016

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 :smile:

avatar ggppdk
ggppdk - comment - 25 Apr 2016

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 ?

avatar Fedik
Fedik - comment - 25 Apr 2016

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)

avatar ggppdk
ggppdk - comment - 25 Apr 2016

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

avatar ggppdk
ggppdk - comment - 25 Apr 2016

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');
avatar Fedik
Fedik - comment - 26 Apr 2016

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);  
}
avatar rdeutz rdeutz - change - 1 May 2016
Milestone Removed:
avatar rdeutz rdeutz - change - 1 May 2016
Milestone Added:
avatar rdeutz rdeutz - change - 1 May 2016
Milestone Added:
avatar rdeutz rdeutz - change - 1 May 2016
Milestone Removed:
avatar ggppdk
ggppdk - comment - 7 May 2016

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
avatar brianteeman brianteeman - change - 9 May 2016
Labels Added: ?

Add a Comment

Login with GitHub to post a comment