From f11cb858be2b32bd02667279751f52a981f50976 Mon Sep 17 00:00:00 2001 From: fw-bot Date: Wed, 30 Oct 2019 11:27:49 +0000 Subject: [PATCH] [FIX] core: remove broken time consuming checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two method valid valid_alternative_icon_text and valid_title_icon represent ~13% of an install all. Rewriting them in master with in #36373 is the main reason of the performance improvement. Those two method logic were broken, because `xpath += '[not(//*[' + valid_attrs_xpath + '])]'` will actually search for valid_attrs_xpath from view root, not from fa- node. -since this check will only log a warning and so only impact bugfix, no impact on user editing views, -since a new check is added in master with the corresponding fix in views, -since this check doesn't really test what it is suppose to check, -since fa accessibility is great, but not critical ->removing those check will slightly speed up build without major impact on views quality closes odoo/odoo#39566 Signed-off-by: Xavier Dollé (xdo) --- .../addons/base/tests/test_view_validation.py | 6 +- odoo/tools/view_validation.py | 82 ------------------- 2 files changed, 1 insertion(+), 87 deletions(-) diff --git a/odoo/addons/base/tests/test_view_validation.py b/odoo/addons/base/tests/test_view_validation.py index c253d1badea..6a9d701cdb3 100644 --- a/odoo/addons/base/tests/test_view_validation.py +++ b/odoo/addons/base/tests/test_view_validation.py @@ -9,7 +9,7 @@ from odoo.tools.view_validation import ( valid_page_in_book, valid_att_in_form, valid_type_in_colspan, valid_type_in_col, valid_att_in_field, valid_att_in_label, valid_field_in_graph, valid_field_in_tree, valid_alternative_image_text, - valid_alternative_icon_text, valid_title_icon, valid_simili_button, + valid_simili_button, valid_simili_progressbar, valid_dialog, valid_simili_dropdown, valid_focusable_button, valid_prohibited_none_role, valid_simili_tabpanel, valid_simili_tab, valid_simili_tablist, valid_alerts @@ -167,10 +167,6 @@ class TestViewValidation(BaseCase): def test_a11y_validation(self): assert valid_alternative_image_text(invalid_form) == "Warning" assert valid_alternative_image_text(valid_form) is True - assert valid_alternative_icon_text(invalid_form) == "Warning" - assert valid_alternative_icon_text(valid_form) is True - assert valid_title_icon(invalid_form) == "Warning" - assert valid_title_icon(valid_form) is True assert valid_simili_button(invalid_form) == "Warning" assert valid_simili_button(valid_form) is True assert valid_dialog(invalid_form) == "Warning" diff --git a/odoo/tools/view_validation.py b/odoo/tools/view_validation.py index a20dc4d46c4..46921fee839 100644 --- a/odoo/tools/view_validation.py +++ b/odoo/tools/view_validation.py @@ -296,88 +296,6 @@ def valid_alternative_image_text(arch, **kwargs): return True -@validate('calendar', 'form', 'graph', 'kanban', 'pivot', 'search', 'tree', 'activity') -def valid_alternative_icon_text(arch, **kwargs): - """An icon with fa- class or in a button must have aria-label in its tag, parents, descendants or have text.""" - valid_aria_attrs = { - 'aria-label', 'aria-labelledby', 't-att-aria-label', 't-attf-aria-label', - 't-att-aria-labelledby', 't-attf-aria-labelledby'} - valid_t_attrs = {'t-value', 't-raw', 't-field', 't-esc'} - valid_attrs = valid_aria_attrs | valid_t_attrs - valid_aria_attrs_xpath = ' or '.join('@' + attr for attr in valid_aria_attrs) - valid_attrs_xpath = ' or '.join('@' + attr for attr in valid_attrs) - - # Select elements with class beginning by 'fa-' - xpath = '(//*[contains(concat(" ", @class), " fa-")' - xpath += ' or contains(concat(" ", @t-att-class), " fa-")' - xpath += ' or contains(concat(" ", @t-attf-class), " fa-")]' - xpath += ' | //button[@icon])' - # Elements with accessibility or string attrs are good - xpath += '[not(' + valid_attrs_xpath + ')]' - # And we ignore all elements with describing in children - xpath += '[not(//*[' + valid_attrs_xpath + '])]' - # Aria label can be on ancestors - xpath += '[not(ancestor[' + valid_aria_attrs_xpath + '])]' - # Labels provide text by definition - xpath += '[not(descendant-or-self::label)]' - # Buttons can have a string attribute - xpath += '[not(descendant-or-self::button[@string])]' - # Fields have labels - xpath += '[not(descendant-or-self::field)]' - # And finally, if there is some text, it's good too - xpath += '[not(descendant-or-self::*[text()])]' - # Following or preceding text - xpath += '[not(preceding-sibling::text()[normalize-space()])]' - xpath += '[not(following-sibling::text()[normalize-space()])]' - # Following or preceding text in span - xpath += '[not(preceding-sibling::span[text()])]' - xpath += '[not(following-sibling::span[text()])]' - - if arch.xpath(xpath): - return "Warning" - return True - - -@validate('calendar', 'form', 'graph', 'kanban', 'pivot', 'search', 'tree', 'activity') -def valid_title_icon(arch, **kwargs): - """An icon with fa- class or in a button must have title in its tag, parents, descendants or have text.""" - valid_title_attrs = {'title', 't-att-title', 't-attf-title'} - valid_t_attrs = {'t-value', 't-raw', 't-field', 't-esc'} - valid_attrs = valid_title_attrs | valid_t_attrs - valid_title_attrs_xpath = ' or '.join('@' + attr for attr in valid_title_attrs) - valid_attrs_xpath = ' or '.join('@' + attr for attr in valid_attrs) - - # Select elements with class beginning by 'fa-' - xpath = '(//*[contains(concat(" ", @class), " fa-")' - xpath += ' or contains(concat(" ", @t-att-class), " fa-")' - xpath += ' or contains(concat(" ", @t-attf-class), " fa-")]' - xpath += ' | //button[@icon])' - # Elements with accessibility or string attrs are good - xpath += '[not(' + valid_attrs_xpath + ')]' - # And we ignore all elements with describing in children - xpath += '[not(//*[' + valid_attrs_xpath + '])]' - # Aria label can be on ancestors - xpath += '[not(ancestor[' + valid_title_attrs_xpath + '])]' - # Labels provide text by definition - xpath += '[not(descendant-or-self::label)]' - # Buttons can have a string attribute - xpath += '[not(descendant-or-self::button[@string])]' - # Fields have labels - xpath += '[not(descendant-or-self::field)]' - # And finally, if there is some text, it's good too - xpath += '[not(descendant-or-self::*[text()])]' - # Following or preceding text - xpath += '[not(preceding-sibling::text()[normalize-space()])]' - xpath += '[not(following-sibling::text()[normalize-space()])]' - # Following or preceding text in span - xpath += '[not(preceding-sibling::span[text()])]' - xpath += '[not(following-sibling::span[text()])]' - - if arch.xpath(xpath): - return "Warning" - return True - - @validate('calendar', 'form', 'graph', 'kanban', 'pivot', 'search', 'tree', 'activity') def valid_simili_button(arch, **kwargs): """A simili button must be tagged with "role='button'"."""