[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#39566 Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
This commit is contained in:
@@ -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', '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'"."""
|
||||
|
||||
Reference in New Issue
Block a user