Skip to content
Snippets Groups Projects
Commit 9f221500 authored by fw-bot's avatar fw-bot Committed by Xavier-Do
Browse files

[FIX] core: remove broken time consuming checks


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#39534

Signed-off-by: default avatarXavier Dollé (xdo) <xdo@odoo.com>
parent 7b3d87b9
Branches
Tags
No related merge requests found
......@@ -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"
......
......@@ -296,88 +296,6 @@ def valid_alternative_image_text(arch, **kwargs):
return True
@validate('calendar', 'diagram', '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', 'diagram', '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', 'diagram', 'form', 'graph', 'kanban', 'pivot', 'search', 'tree', 'activity')
def valid_simili_button(arch, **kwargs):
"""A simili button must be tagged with "role='button'"."""
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment