[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 #27933

addons/website/models/ir_qweb.py

@@ -12,11 +12,11 @@
 
 
 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
 
 

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

odoo/addons/base/models/assetsbundle.py

@@ -27,8 +27,6 @@
 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 @@ def checksum(self):
         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 @@ def get_attachments(self, type, ignore_version=False):
             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,19 +266,15 @@ def get_attachments(self, type, ignore_version=False):
         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']
 
         # Set user direction in name to store two bundles
         # 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 @@ def save_attachment(self, type, content, inc=None):
             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 @@ def css(self):
             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
 

odoo/addons/test_assetsbundle/tests/test_assetsbundle.py

@@ -46,9 +46,9 @@ def _any_ira_for_bundle(self, type, lang=None):
         """
         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 @@ def test_05_debug(self):
         # 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
-        # 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):
+    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': 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)
-
-    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)
+        self.assertEquals(date0, date1)
 
-    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 = """
         <data>
@@ -265,7 +219,7 @@ def test_11_paginated_css_content_invalidation(self):
             '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,28 +228,19 @@ def test_11_paginated_css_content_invalidation(self):
         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)
 
         # 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 @@ def test_14_duplicated_css_assets(self):
 
         # 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