{"action":"created","issue":{"url":"https://api.github.com/repos/joomla/joomla-cms/issues/10136","repository_url":"https://api.github.com/repos/joomla/joomla-cms","labels_url":"https://api.github.com/repos/joomla/joomla-cms/issues/10136/labels{/name}","comments_url":"https://api.github.com/repos/joomla/joomla-cms/issues/10136/comments","events_url":"https://api.github.com/repos/joomla/joomla-cms/issues/10136/events","html_url":"https://github.com/joomla/joomla-cms/pull/10136","id":151967854,"number":10136,"title":"Fix findLabel() performance being bad if labels are missing and being O(n^n) when they don't have tag id like ID-lbl","user":{"login":"ggppdk","id":5092940,"avatar_url":"https://avatars.githubusercontent.com/u/5092940?v=3","gravatar_id":"","url":"https://api.github.com/users/ggppdk","html_url":"https://github.com/ggppdk","followers_url":"https://api.github.com/users/ggppdk/followers","following_url":"https://api.github.com/users/ggppdk/following{/other_user}","gists_url":"https://api.github.com/users/ggppdk/gists{/gist_id}","starred_url":"https://api.github.com/users/ggppdk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ggppdk/subscriptions","organizations_url":"https://api.github.com/users/ggppdk/orgs","repos_url":"https://api.github.com/users/ggppdk/repos","events_url":"https://api.github.com/users/ggppdk/events{/privacy}","received_events_url":"https://api.github.com/users/ggppdk/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/joomla/joomla-cms/labels/PR-staging","name":"PR-staging","color":"c7def8"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":9,"created_at":"2016-04-29T21:00:11Z","updated_at":"2016-05-02T17:06:34Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/joomla/joomla-cms/pulls/10136","html_url":"https://github.com/joomla/joomla-cms/pull/10136","diff_url":"https://github.com/joomla/joomla-cms/pull/10136.diff","patch_url":"https://github.com/joomla/joomla-cms/pull/10136.patch"},"body":"#### Summary of Changes\r\nIf you have a lot of form elements e.g. buttons that do not have a label or if the form has labels that do not have HTML tag ID like *-lbl, then findLabel() performance does a full DOM search using selector:\r\nvar $label = $('label[for=\"'+$elem.attr('id')+'\"]');\r\n\r\nThe above describe findLabel() behaviour of:\r\n- validate.js\r\n\r\nFor:\r\n- html5fallback.js\r\nthe full DOM search is always performed\r\n\r\n\r\n#### Testing Instructions\r\nApply the patch the edit file (or some other form file):\r\nand replace:\r\n```\r\n</form>\r\n```\r\n\r\nwith:\r\n```\r\n<?php\r\necho '\r\n<script>\r\n\tvar test_cnt = 0;\r\n\t\r\n\tfunction time_test_Isvalid(el)\r\n\t{\r\n\t\twindow.console.log( \\'CALLING isValid()\\' );\r\n\t\t\r\n\t\tvar vTimeStart = new Date();\r\n\t\tdocument.formvalidator.isValid(document.getElementById(el.form.id));\r\n\t\tvar vTimeDiff = (new Date())  - vTimeStart;\r\n\t\t\r\n\t\twindow.console.log( \\'isValid() time: \\' + vTimeDiff );\r\n\t\t\r\n\t\treturn false;\r\n\t}\r\n\t\r\n\tfunction inject_test_input(el, box_id)\r\n\t{\r\n\t\tvar box = document.getElementById(box_id);\r\n\t\t\r\n\t\tvar lbl = document.createElement(\\'label\\');\r\n\t\tlbl.setAttribute(\\'for\\', \\'testid_\\' + test_cnt);\r\n\t\tlbl.setAttribute(\\'class\\', \\'label\\');\r\n\t\tlbl.innerHTML = \\'New element\\';\r\n\t\tbox.appendChild(lbl);\r\n\t\t\r\n\t\tvar input = document.createElement(\\'input\\');\r\n\t\tinput.type = \\'text\\';\r\n\t\tinput.setAttribute(\\'class\\', \\'required\\');\r\n\t\tinput.id = \\'testid_\\' + test_cnt;\r\n\t\tbox.appendChild(input);\r\n\t\t\r\n\t\ttest_cnt++;\r\n\t\twindow.console.log(\\'Element injected\\');\r\n\t\treturn false;\r\n\t}\t\r\n</script>\r\n\r\n<br/>\r\n<button  onclick=\"time_test_Isvalid(this);  return false;\"  class=\"btn\">  Test validation </button>\r\n<button  onclick=\"inject_test_input(this, \\'inject_test_box\\'); time_test_Isvalid(this); return false;\"  class=\"btn\">\tInject field + Test validation </button>\r\n<div id=\"inject_test_box\"></div>\r\n';\r\n$use_labels = 1;\r\n$add_for = 1;\r\n$add_dash_lbl = 1;\r\nfor($i = 0; $i < 10000; $i++)\r\n{\r\n\tif ($use_labels)\r\n\t{\r\n\t\t$lbl_for = $add_for  ?  'for=\"dummy_field_'.$i.'\"'  :  '';\r\n\t\t$lbl_id  = $add_dash_lbl  ?  'id=\"dummy_field_'.$i.'-lbl\"'  :  'id=\"dummy_field_'.$i.'-nodashlbl\"';\r\n\t\techo '<label '.$lbl_for.' '.$lbl_id.' class=\"label\">Element '.$i.'</label> ';\r\n\t}\r\n\techo '<input id=\"dummy_field_'.$i.'\" type=\"text\" name=\"dummy_field_'.$i.'\" /> ';\r\n}\r\n?>\r\n\r\n</form>\r\n```\r\n\r\nThe above will \r\n- add 10,000 form elements into the form\r\n- add a button to trigger and TIME validation of isValid() (validate.js) without submitting the form\r\n(open console to see output)\r\n- add a button to inject a new form element into the DOM and re-trigger validation, so that we can verify that validation behaviour and performance after validation has been trigger once and some element has been added after it\r\n\r\n**Validations** (testing times incude only isValid() of validate.js):\r\n```\r\nAll fields have labels with ID-lbl\r\n             1st      2nd, +           After Injecting new field\r\nBEFORE:      1070ms    about 250ms      250ms - 550ms\r\nAFTER:        630ms    about 250ms      370ms - 630ms\r\n\r\nAll 10,000 labels are missing, e.g. input buttons or buttons\r\n             1st      2nd, +           After Injecting new field\r\nBEFORE:     5700ms   about 180ms    180ms - 430ms\r\nAFTER:       450ms   about 180ms    230ms - 480ms\r\n\r\n\r\nAll 10,000 labels don't have id like ID-lbl\r\n             1st       2nd, +         After Injecting new field\r\nBEFORE:   28700ms    about 220ms      220ms - 440ms\r\nAFTER:      620ms    about 220ms      360ms - 650ms\r\n```"},"comment":{"url":"https://api.github.com/repos/joomla/joomla-cms/issues/comments/216294269","html_url":"https://github.com/joomla/joomla-cms/pull/10136#issuecomment-216294269","issue_url":"https://api.github.com/repos/joomla/joomla-cms/issues/10136","id":216294269,"user":{"login":"ggppdk","id":5092940,"avatar_url":"https://avatars.githubusercontent.com/u/5092940?v=3","gravatar_id":"","url":"https://api.github.com/users/ggppdk","html_url":"https://github.com/ggppdk","followers_url":"https://api.github.com/users/ggppdk/followers","following_url":"https://api.github.com/users/ggppdk/following{/other_user}","gists_url":"https://api.github.com/users/ggppdk/gists{/gist_id}","starred_url":"https://api.github.com/users/ggppdk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ggppdk/subscriptions","organizations_url":"https://api.github.com/users/ggppdk/orgs","repos_url":"https://api.github.com/users/ggppdk/repos","events_url":"https://api.github.com/users/ggppdk/events{/privacy}","received_events_url":"https://api.github.com/users/ggppdk/received_events","type":"User","site_admin":false},"created_at":"2016-05-02T17:06:34Z","updated_at":"2016-05-02T17:06:34Z","body":"@Fedik \r\n\r\ni have changed the\r\nelem.data('label') to store **false** if label is missing **as you had suggested**\r\n- 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,\r\n\r\nTo summarize new changes / fixes\r\n1. now it is doing both:\r\n-- setting elem.data('label') to **false** if label is missing \r\n-- 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())\r\n2. 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\r\n3. 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"}}