From a8cec47c930c1d3b60f0e5c5788106cc30dc1a0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 17 Oct 2018 12:35:03 +0000 Subject: [PATCH] [IMP] assets: remove css files split Before this commit, our asset bundles had a special behaviour for bundles of type css: because of an old limitation in IE9, css files had to be split to make sure they had less than 4095 rules per stylesheets. Since we no longer support IE9, this code can be removed, which should slightly simplify the bundle code, and should slightly reduce the number of requests required to properly load odoo. closes odoo/odoo#27933 --- addons/website/models/ir_qweb.py | 4 +- doc/reference/javascript_reference.rst | 5 +- odoo/addons/base/models/assetsbundle.py | 49 +++------ .../tests/test_assetsbundle.py | 99 +++++-------------- 4 files changed, 37 insertions(+), 120 deletions(-) diff --git a/addons/website/models/ir_qweb.py b/addons/website/models/ir_qweb.py index 32d221ea5ea..0a36e8fa8ee 100644 --- a/addons/website/models/ir_qweb.py +++ b/addons/website/models/ir_qweb.py @@ -12,11 +12,11 @@ re_background_image = re.compile(r"(background-image\s*:\s*url\(\s*['\"]?\s*)([^ class AssetsBundleMultiWebsite(AssetsBundle): - def _get_asset_url_values(self, id, unique, extra, name, page, type): + def _get_asset_url_values(self, id, unique, extra, name, sep, type): website_id = self.env.context.get('website_id') website_id_path = website_id and ('%s/' % website_id) or '' extra = website_id_path + extra - res = super(AssetsBundleMultiWebsite, self)._get_asset_url_values(id, unique, extra, name, page, type) + res = super(AssetsBundleMultiWebsite, self)._get_asset_url_values(id, unique, extra, name, sep, type) return res diff --git a/doc/reference/javascript_reference.rst b/doc/reference/javascript_reference.rst index fe04dc06a6f..034938acaaf 100644 --- a/doc/reference/javascript_reference.rst +++ b/doc/reference/javascript_reference.rst @@ -129,9 +129,8 @@ Here is what happens when a template is rendered by the server with these direct be replaced by a list of script tags pointing to the js files - if we are not in *debug=assets* mode, - - the css files will be concatenated and minified, then splits into files - with no more than 4096 rules (to get around an old limitation of IE9). Then, - we generate as many stylesheet tags as necessary + - the css files will be concatenated and minified, then a stylesheet tag is + generated - the js files are concatenated and minified, then a script tag is generated Note that the assets files are cached, so in theory, a browser should only load diff --git a/odoo/addons/base/models/assetsbundle.py b/odoo/addons/base/models/assetsbundle.py index abdb09b0d13..a2170ec1af8 100644 --- a/odoo/addons/base/models/assetsbundle.py +++ b/odoo/addons/base/models/assetsbundle.py @@ -27,8 +27,6 @@ from odoo.tools import func, misc import logging _logger = logging.getLogger(__name__) -MAX_CSS_RULES = 4095 - class CompileError(RuntimeError): pass def rjsmin(script): @@ -92,7 +90,6 @@ class AssetsBundle(object): def __init__(self, name, files, remains=None, env=None): self.name = name self.env = request.env if env is None else env - self.max_css_rules = self.env.context.get('max_css_rules', MAX_CSS_RULES) self.javascripts = [] self.stylesheets = [] self.css_errors = [] @@ -199,31 +196,27 @@ class AssetsBundle(object): return hashlib.sha1(check.encode('utf-8')).hexdigest() def _get_asset_template_url(self): - return "/web/content/{id}-{unique}/{extra}{name}{page}{type}" # name contains inc + return "/web/content/{id}-{unique}/{extra}{name}{sep}{type}" - def _get_asset_url_values(self, id, unique, extra, name, page, type): # extra can contain direction or/and website + def _get_asset_url_values(self, id, unique, extra, name, sep, type): # extra can contain direction or/and website return { 'id': id, 'unique': unique, 'extra': extra, 'name': name, - 'page': page, + 'sep': sep, 'type': type, } - def get_asset_url(self, id='%', unique='%', extra='', name='%', page='%', type='%'): + def get_asset_url(self, id='%', unique='%', extra='', name='%', sep="%", type='%'): return self._get_asset_template_url().format( - **self._get_asset_url_values(id=id, unique=unique, extra=extra, name=name, page=page, type=type) + **self._get_asset_url_values(id=id, unique=unique, extra=extra, name=name, sep=sep, type=type) ) def clean_attachments(self, type): """ Takes care of deleting any outdated ir.attachment records associated to a bundle before saving a fresh one. - When `type` is css we need to check that we are deleting a different version (and not *any* - version) because css may be paginated and, therefore, may produce multiple attachments for - the same bundle's version. - When `type` is js we need to check that we are deleting a different version (and not *any* version) because, as one of the creates in `save_attachment` can trigger a rollback, the call to `clean_attachments ` is made at the end of the method in order to avoid the rollback @@ -259,7 +252,7 @@ class AssetsBundle(object): unique=unique, extra='%s' % ('rtl/' if type == 'css' and self.user_direction == 'rtl' else ''), name=self.name, - page='.%' if type == 'css' else '', + sep='', type='.%s' % type ) self.env.cr.execute(""" @@ -273,7 +266,7 @@ class AssetsBundle(object): attachment_ids = [r[0] for r in self.env.cr.fetchall()] return self.env['ir.attachment'].sudo().browse(attachment_ids) - def save_attachment(self, type, content, inc=None): + def save_attachment(self, type, content): assert type in ('js', 'css') ira = self.env['ir.attachment'] @@ -281,11 +274,7 @@ class AssetsBundle(object): # 1 for ltr and 1 for rtl, this will help during cleaning of assets bundle # and allow to only clear the current direction bundle # (this applies to css bundles only) - fname = '%s%s.%s' % ( - self.name, - ('' if inc is None else '.%s' % inc), - type - ) + fname = '%s.%s' % (self.name, type) mimetype = 'application/javascript' if type == 'js' else 'text/css' values = { 'name': "/web/content/%s" % type, @@ -304,8 +293,8 @@ class AssetsBundle(object): unique=self.version, extra='%s' % ('rtl/' if type == 'css' and self.user_direction == 'rtl' else ''), name=fname, - page='', # included in fname - type='' # included in fname + sep='', # included in fname + type='' ) values = { 'name': url, @@ -341,23 +330,7 @@ class AssetsBundle(object): matches.append(css) css = u'\n'.join(matches) - # split for browser max file size and browser max expression - re_rules = '([^{]+\{(?:[^{}]|\{[^{}]*\})*\})' - re_selectors = '()(?:\s*@media\s*[^{]*\{)?(?:\s*(?:[^,{]*(?:,|\{(?:[^}]*\}))))' - page = [] - pages = [page] - page_selectors = 0 - for rule in re.findall(re_rules, css): - selectors = len(re.findall(re_selectors, rule)) - if page_selectors + selectors <= self.max_css_rules: - page_selectors += selectors - page.append(rule) - else: - pages.append([rule]) - page = pages[-1] - page_selectors = selectors - for idx, page in enumerate(pages): - self.save_attachment("css", ' '.join(page), inc=idx) + self.save_attachment("css", css) attachments = self.get_attachments('css') return attachments diff --git a/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py b/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py index a2b03940e2b..983f6a87d7f 100644 --- a/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py +++ b/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py @@ -46,9 +46,9 @@ class TestJavascriptAssetsBundle(TransactionCase): """ user_direction = self.env['res.lang'].search([('code', '=', (lang or self.env.user.lang))]).direction bundle = self.jsbundle_xmlid if type == 'js' else self.cssbundle_xmlid - return self.env['ir.attachment'].search([ - ('url', '=like', '/web/content/%-%/{0}{1}%.{2}'.format(('rtl/' if type == 'css' and user_direction == 'rtl' else ''), bundle, type)) - ]) + url = '/web/content/%-%/{0}{1}.{2}'.format(('rtl/' if type == 'css' and user_direction == 'rtl' else ''), bundle, type) + domain = [('url', '=like', url)] + return self.env['ir.attachment'].search(domain) def test_01_generation(self): """ Checks that a bundle creates an ir.attachment record when its `js` method is called @@ -161,94 +161,48 @@ class TestJavascriptAssetsBundle(TransactionCase): # there shouldn't be any assets created in debug mode self.assertEquals(len(self._any_ira_for_bundle('js')), 0) - def test_06_paginated_css_generation1(self): - """ Checks that a bundle creates enough ir.attachment records when its `css` method is called - for the first time while the number of css rules exceed the limit. - """ - # note: changing the max_css_rules of a bundle does not invalidate its attachments + def test_08_css_generation3(self): # self.cssbundle_xlmid contains 3 rules - self.bundle = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 1})) - self.bundle.css() - self.assertEquals(len(self._any_ira_for_bundle('css')), 3) - self.assertEquals(len(self.bundle.get_attachments('css')), 3) - - def test_07_paginated_css_generation2(self): - # self.cssbundle_xlmid contains 3 rules - self.bundle = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 2})) - self.bundle.css() - self.assertEquals(len(self._any_ira_for_bundle('css')), 2) - self.assertEquals(len(self.bundle.get_attachments('css')), 2) - - def test_08_paginated_css_generation3(self): - # self.cssbundle_xlmid contains 3 rules - self.bundle = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 3})) + self.bundle = self._get_asset(self.cssbundle_xmlid) self.bundle.css() self.assertEquals(len(self._any_ira_for_bundle('css')), 1) self.assertEquals(len(self.bundle.get_attachments('css')), 1) - def test_09_paginated_css_access(self): + def test_09_css_access(self): """ Checks that the bundle's cache is working, i.e. that a bundle creates only enough ir.attachment records when rendered multiple times. """ - bundle0 = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 1})) + bundle0 = self._get_asset(self.cssbundle_xmlid) bundle0.css() - self.assertEquals(len(self._any_ira_for_bundle('css')), 3) + self.assertEquals(len(self._any_ira_for_bundle('css')), 1) version0 = bundle0.version - ira0, ira1, ira2 = self._any_ira_for_bundle('css') + ira0 = self._any_ira_for_bundle('css') date0 = ira0.create_date - date1 = ira1.create_date - date2 = ira2.create_date - bundle1 = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 1})) + bundle1 = self._get_asset(self.cssbundle_xmlid) bundle1.css() - self.assertEquals(len(self._any_ira_for_bundle('css')), 3) + self.assertEquals(len(self._any_ira_for_bundle('css')), 1) version1 = bundle1.version - ira3, ira4, ira5 = self._any_ira_for_bundle('css') - date3 = ira1.create_date - date4 = ira1.create_date - date5 = ira1.create_date + ira1 = self._any_ira_for_bundle('css') + date1 = ira1.create_date self.assertEquals(version0, version1) - self.assertEquals(date0, date3) - self.assertEquals(date1, date4) - self.assertEquals(date2, date5) + self.assertEquals(date0, date1) - def test_10_paginated_css_date_invalidation(self): - """ Checks that a bundle is invalidated when one of its assets' modification date is changed. - """ - bundle0 = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 1})) - bundle0.css() - last_modified0 = bundle0.last_modified - version0 = bundle0.version - - path = get_resource_path('test_assetsbundle', 'static', 'src', 'css', 'test_cssfile1.css') - bundle1 = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 1})) - _touch(path, bundle1) - - bundle1.css() - last_modified1 = bundle1.last_modified - version1 = bundle1.version - - self.assertNotEquals(last_modified0, last_modified1) - self.assertNotEquals(version0, version1) - - # check if the previous attachment is correctly cleaned - self.assertEquals(len(self._any_ira_for_bundle('css')), 3) - - def test_11_paginated_css_content_invalidation(self): + def test_11_css_content_invalidation(self): """ Checks that a bundle is invalidated when its content is modified by adding a file to source. """ - bundle0 = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 1})) + bundle0 = self._get_asset(self.cssbundle_xmlid) bundle0.css() files0 = bundle0.files version0 = bundle0.version - self.assertEquals(len(self._any_ira_for_bundle('css')), 3) + self.assertEquals(len(self._any_ira_for_bundle('css')), 1) view_arch = """ @@ -265,7 +219,7 @@ class TestJavascriptAssetsBundle(TransactionCase): 'inherit_id': bundle.id, }) - bundle1 = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'check_view_ids': view.ids, 'max_css_rules': 1})) + bundle1 = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'check_view_ids': view.ids})) bundle1.css() files1 = bundle1.files version1 = bundle1.version @@ -274,12 +228,12 @@ class TestJavascriptAssetsBundle(TransactionCase): self.assertNotEquals(version0, version1) # check if the previous attachment are correctly cleaned - self.assertEquals(len(self._any_ira_for_bundle('css')), 4) + self.assertEquals(len(self._any_ira_for_bundle('css')), 1) - def test_12_paginated_css_debug(self): + def test_12_css_debug(self): """ Check that a bundle in debug mode outputs non-minified assets. """ - debug_bundle = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 1})) + debug_bundle = self._get_asset(self.cssbundle_xmlid) content = debug_bundle.to_html(debug='assets') # find back one of the original asset file self.assertIn('/test_assetsbundle/static/src/css/test_cssfile1.css', content) @@ -287,15 +241,6 @@ class TestJavascriptAssetsBundle(TransactionCase): # there shouldn't be any assets created in debug mode self.assertEquals(len(self._any_ira_for_bundle('css')), 0) - def test_13_paginated_css_order(self): - # self.cssbundle_xlmid contains 3 rules - self.bundle = self._get_asset(self.cssbundle_xmlid, env=self.env(context={'max_css_rules': 1})) - stylesheets = self.bundle.css() - - self.assertTrue(stylesheets[0].url.endswith('.0.css')) - self.assertTrue(stylesheets[1].url.endswith('.1.css')) - self.assertTrue(stylesheets[2].url.endswith('.2.css')) - def test_14_duplicated_css_assets(self): """ Checks that if the bundle's ir.attachment record is duplicated, the bundle is only sourced once. This could happen if multiple transactions try to render the bundle simultaneously. @@ -312,7 +257,7 @@ class TestJavascriptAssetsBundle(TransactionCase): # the ir.attachment records should be deduplicated in the bundle's content content = bundle0.to_html() - self.assertEqual(content.count('test_assetsbundle.bundle2.0.css'), 1) + self.assertEqual(content.count('test_assetsbundle.bundle2.css'), 1) # Language direction specific tests