[IMP] base: improve css compilation error reporting

The current solution returns two node, one for the css itself, another
for the script managing the error. This means that we must know if we
have a css error when generating the assets links, when rendering a
t-call-assets.

The proposed solution will solve this issue by managing the error
in the css file itself. A slight change will also save the error in the
attachment instead of choosing the previous one, meaning that the
error will be saved and the bundle won't be recomputed anymore.

This solution also removes the only case when a t-call-assets can return
content instead of a link, this will be cleaned in the next commit.

As a slight change, a warning will alway be displayed on the bottom of
the page, to manage the case where the javascript managing the error is
not in the page.

Part-of: odoo/odoo#131353

addons/web/static/src/core/errors/error_dialogs.xml

@@ -85,7 +85,7 @@
           <pre t-esc="props.message"/>
         </div>
         <t t-set-slot="footer">
-          <button class="btn btn-primary o-default-button" t-on-click="onClick">Close</button>
+          <button class="btn btn-primary o-default-button" t-on-click="props.close">Close</button>
         </t>
       </Dialog>
     </t>

addons/web/static/src/core/errors/scss_error_dialog.js

@@ -13,11 +13,16 @@ ScssErrorDialog.title = _t("Style error");
 const scssErrorDisplayService = {
     dependencies: ["dialog"],
     start(env, { dialog }) {
-        if (window.__odooScssCompilationError) {
-            dialog.add(ScssErrorDialog, {
-                message: window.__odooScssCompilationError,
-            });
+        const assets = [...document.styleSheets].filter((sheet) => sheet.href?.includes("/web/assets"));
+        for (const { cssRules } of assets) {
+            const lastRule = cssRules?.[cssRules?.length - 1];
+            if (lastRule?.selectorText === "css_error_message") {
+                dialog.add(ScssErrorDialog, {
+                    message: lastRule.style.content.replaceAll("\\a", "\n").replaceAll("\\*", "*").replaceAll(`\\"`, `"`),
+                });
+            }
         }
+
     },
 };
 

addons/web/static/src/start.js

@@ -26,13 +26,13 @@ export async function startWebClient(Webclient) {
     odoo.isReady = false;
 
     // setup environment
+    await whenReady();
     const env = makeEnv();
     await startServices(env);
 
     Component.env = env;
 
     // start web client
-    await whenReady();
     const app = new App(Webclient, {
         name: "Odoo Web Client",
         env,

odoo/addons/base/models/assetsbundle.py

@@ -115,33 +115,18 @@ class AssetsBundle(object):
         """
         response = []
         if self.has_css and self.stylesheets:
-            css_attachments = self.css(is_minified=not self.is_debug_assets) or []
-            for attachment in css_attachments:
-                if self.is_debug_assets:
-                    href = self.get_debug_asset_url(extra=self.extra('css'),
-                                                    name=css_attachments.name,
-                                                    extension='')
-                else:
-                    href = attachment.url
-                response.append((href, None))
-            if self.css_errors:
-                msg = '\n'.join(self.css_errors).replace('"', '\\"').replace('\n', '\\n')
-                js_error = """
-                    window.__odooScssCompilationError = "%s";
-                    console.error("SCSS compilation failure:", window.__odooScssCompilationError);
-                    window.addEventListener("DOMContentLoaded", () => {
-                        if (!odoo || !odoo.define) {
-                            alert(window.__odooScssCompilationError);
-                        }
-                    });
-                """ % msg
-                response.append((None, js_error))
-                response.append(('/web/static/lib/bootstrap/dist/css/bootstrap.css', None))
+            css_attachment = self.css(is_minified=not self.is_debug_assets)
+            if self.is_debug_assets:
+                response.append((self.get_debug_asset_url(extra=self.extra('css'), name=css_attachment.name, extension=''), None))
+            else:
+                response.append((css_attachment.url, None))
 
         if self.has_js and self.javascripts:
             js_attachment = self.js(is_minified=not self.is_debug_assets)
-            src = self.get_debug_asset_url(name=js_attachment.name, extension='') if self.is_debug_assets else js_attachment[0].url
-            response.append((src, None))
+            if self.is_debug_assets:
+                response.append((self.get_debug_asset_url(extra=self.extra('js'), name=js_attachment.name, extension=''), None))
+            else:
+                response.append((js_attachment.url, None))
 
         return self.external_assets + response
 
@@ -578,25 +563,47 @@ class AssetsBundle(object):
     def css(self, is_minified=True):
         extension = 'min.css' if is_minified else 'css'
         attachments = self.get_attachments(extension)
-        if not attachments:
-            # get css content
-            css = self.preprocess_css()
-            if self.css_errors:
-                return self.get_attachments(extension, ignore_version=True)
+        if attachments:
+            return attachments
 
-            matches = []
-            css = re.sub(self.rx_css_import, lambda matchobj: matches.append(matchobj.group(0)) and '', css)
+        css = self.preprocess_css()
+        if self.css_errors:
+            error_message = '\n'.join(self.css_errors).replace('"', '\\"').replace('\n', '\A').replace('*', '\*')
+            previous_attachment = self.get_attachments(extension, ignore_version=True)
+            previous_css = previous_attachment.raw.decode() if previous_attachment else ''
+            css_error_message_header = '\n\n/* ## CSS error message ##*/'
+            previous_css = previous_css.split(css_error_message_header)[0]
+            css = css_error_message_header.join([
+                previous_css, """
+body::before {
+  font-weight: bold;
+  content: "A css error occured, using an old style to render this page";
+  position: fixed;
+  left: 0;
+  bottom: 0;
+  z-index: 100000000000;
+  background-color: #C00;
+  color: #DDD;
+}
 
-            if is_minified:
-                # move up all @import rules to the top
-                matches.append(css)
-                css = u'\n'.join(matches)
+css_error_message {
+  content: "%s";
+}
+""" % error_message
+            ])
+            return self.save_attachment(extension, css)
 
-                self.save_attachment(extension, css)
-                attachments = self.get_attachments(extension)
-            else:
-                return self.css_with_sourcemap(u'\n'.join(matches))
-        return attachments
+        matches = []
+        css = re.sub(self.rx_css_import, lambda matchobj: matches.append(matchobj.group(0)) and '', css)
+
+        if is_minified:
+            # move up all @import rules to the top
+            matches.append(css)
+            css = u'\n'.join(matches)
+
+            return self.save_attachment(extension, css)
+        else:
+            return self.css_with_sourcemap(u'\n'.join(matches))
 
     def css_with_sourcemap(self, content_import_rules):
         """Create the ir.attachment representing the not-minified content of the bundleCSS

odoo/addons/test_assetsbundle/__manifest__.py

@@ -12,6 +12,9 @@
     ],
 
     'assets': {
+        'web.assets_tests': [
+            'test_assetsbundle/static/tests/test_css_error.js',
+        ],
         'test_assetsbundle.bundle2': [
             'test_assetsbundle/static/src/css/test_cssfile1.css',
         ],

odoo/addons/test_assetsbundle/static/src/css/test_error.scss

@@ -0,0 +1,3 @@
+.rule1 ()){ /* error */
+    color: black;
+}

odoo/addons/test_assetsbundle/static/tests/test_css_error.js

@@ -0,0 +1,28 @@
+/** @odoo-module **/
+
+import { registry } from "@web/core/registry";
+
+registry.category("web_tour.tours").add('css_error_tour', {
+    test: true,
+    url: '/web',
+    steps: () => [
+    {
+        content: "Error message",
+        trigger: ".modal-body",
+        run: () => {},
+    },
+]});
+
+
+registry.category("web_tour.tours").add('css_error_tour_frontend', {
+    test: true,
+    url: '/',
+    steps: () => [
+    {
+        content: "Error message",
+        trigger: ".modal-body",
+        run: () => {},
+    },
+]});
+
+// Note, the ideal steap would be `.modal-body:contains('Error: Invalid CSS after \".rule1\": expected selector, was \"()){ /* error */')` but it fails sometimes

odoo/addons/test_assetsbundle/tests/test_assetsbundle.py

@@ -2028,3 +2028,26 @@ class AssetsNodeOrmCacheUsage(TransactionCase):
         asset_keys, qweb_keys = self.cache_keys()
         self.assertEqual(len(asset_keys), 1, "lazy_load shouldn't create another entry")
         self.assertEqual(len(qweb_keys), 1, "lazy_load shouldn't create another entry")
+
+@tagged('-at_install', 'post_install')
+class TestErrorManagement(HttpCase):
+
+    def test_assets_bundle_css_error_backend(self):
+        self.env['ir.qweb']._get_asset_bundle('web.assets_backend', assets_params={'website_id': self.env['website'].search([], limit=1).id}).css() # force pregeneration so that we have the base style
+        self.env['ir.asset'].create({
+            'name': 'Css error',
+            'bundle': 'web.assets_backend',
+            'path': 'test_assetsbundle/static/src/css/test_error.scss',
+        })
+
+        with mute_logger('odoo.addons.base.models.assetsbundle'):
+            self.start_tour('/web', 'css_error_tour', login='admin')
+
+    def test_assets_bundle_css_error_frontend(self):
+        self.env['ir.asset'].create({
+            'name': 'Css error',
+            'bundle': 'web.assets_frontend',
+            'path': 'test_assetsbundle/static/src/css/test_error.scss',
+        })
+        with mute_logger('odoo.addons.base.models.assetsbundle'):
+            self.start_tour('/', 'css_error_tour_frontend', login='admin')