diff --git a/Products/CMFPlone/resources/browser/resource.py b/Products/CMFPlone/resources/browser/resource.py index aba363c73d..9124982bad 100644 --- a/Products/CMFPlone/resources/browser/resource.py +++ b/Products/CMFPlone/resources/browser/resource.py @@ -107,7 +107,9 @@ def check_dependencies(bundle_name, depends, bundles): valid_dependencies = [] for name in depend_names: - if name in bundles: + if name in bundles or name == "all": + # A valid dependency or special dependency "all", which + # specifies that the bundle should be rendered last. valid_dependencies.append(name) continue if name in all_names: @@ -133,7 +135,13 @@ def check_dependencies(bundle_name, depends, bundles): return valid_dependencies - # register + # Helper vars for the `all` dependency. + last_js_bundle = None + last_css_bundle = None + depends_all_js = [] + depends_all_css = [] + + # Register resources for name, record in records.items(): include = record.enabled include = include or name in theme_enabled_bundles @@ -146,7 +154,7 @@ def check_dependencies(bundle_name, depends, bundles): if depends == "__broken__": continue external = self.is_external_url(record.jscompilation) - PloneScriptResource( + resource = PloneScriptResource( context=self.context, name=name, depends=depends, @@ -163,12 +171,19 @@ def check_dependencies(bundle_name, depends, bundles): integrity=not external, **{"data-bundle": name}, ) + if "all" in depends: + # Remember the JS bundles that should be rendered last + depends_all_js.append(resource) + else: + # Remember the last JS bundle that was added and not be + # rendered last. + last_js_bundle = name if record.csscompilation: depends = check_dependencies(name, record.depends, css_names) if depends == "__broken__": continue external = self.is_external_url(record.csscompilation) - PloneStyleResource( + resource = PloneStyleResource( context=self.context, name=name, depends=depends, @@ -183,6 +198,13 @@ def check_dependencies(bundle_name, depends, bundles): rel="stylesheet", **{"data-bundle": name}, ) + if "all" in depends: + # Remember the JS bundles that should be rendered last + depends_all_css.append(resource) + else: + # Remember the last JS bundle that was added and not be + # rendered last. + last_css_bundle = name # Collect theme data themedata = {} @@ -213,6 +235,7 @@ def check_dependencies(bundle_name, depends, bundles): integrity=not external, **{"data-bundle": "diazo"}, ) + last_js_bundle = "theme" # add Theme CSS if themedata["production_css"]: @@ -236,8 +259,29 @@ def check_dependencies(bundle_name, depends, bundles): rel="stylesheet", **{"data-bundle": "diazo"}, ) + last_css_bundle = "theme" + + # Fix "all" dependencies to be rendered after the last known non-all resource. + for resource in depends_all_js: + # Change all "all" to last known JS resource + dependencies = map( + lambda dep: dep if dep != "all" else last_js_bundle, resource.depends + ) + # Remove None values, e.g. if no known JS resource was found + dependencies = filter(lambda dep: dep is not None, dependencies) + resource.depends = list(dependencies) + + for resource in depends_all_css: + # Change all "all" to last known JS resource + dependencies = map( + lambda dep: dep if dep != "all" else last_css_bundle, resource.depends + ) + # Remove None values, e.g. if no known JS resource was found + dependencies = filter(lambda dep: dep is not None, dependencies) + resource.depends = list(dependencies) - # add Custom CSS + # Add Custom CSS + # This one is always rendered last. registry = getUtility(IRegistry) theme_settings = registry.forInterface(IThemeSettings, False) if theme_settings.custom_css: diff --git a/Products/CMFPlone/tests/testResourceRegistries.py b/Products/CMFPlone/tests/testResourceRegistries.py index f33fa292bc..496dcb293c 100644 --- a/Products/CMFPlone/tests/testResourceRegistries.py +++ b/Products/CMFPlone/tests/testResourceRegistries.py @@ -1,3 +1,4 @@ +from lxml import etree from OFS.Image import File from plone.app.testing import logout from plone.app.testing import setRoles @@ -19,22 +20,24 @@ from zope.component import getUtility -class TestScriptsViewlet(PloneTestCase.PloneTestCase): - def _make_test_bundle(self): - registry = getUtility(IRegistry) +def _make_test_bundle( + name="foobar", + depends="", +): + registry = getUtility(IRegistry) + + bundles = registry.collectionOfInterface(IBundleRegistry, prefix="plone.bundles") + bundle = bundles.add(name) + bundle.name = name + bundle.jscompilation = f"http://foo.bar/{name}.js" + bundle.csscompilation = f"http://foo.bar/{name}.css" + bundle.depends = depends + return bundle - bundles = registry.collectionOfInterface( - IBundleRegistry, prefix="plone.bundles" - ) - bundle = bundles.add("foobar") - bundle.name = "foobar" - bundle.jscompilation = "http://foo.bar/foobar.js" - bundle.csscompilation = "http://foo.bar/foobar.css" - bundle.resources = ["foobar"] - return bundle +class TestScriptsViewlet(PloneTestCase.PloneTestCase): def test_bundle_defernot_asyncnot(self): - self._make_test_bundle() + _make_test_bundle() view = ScriptsView(self.app, self.app.REQUEST, None, None) view.update() rendered = view.render() @@ -42,7 +45,7 @@ def test_bundle_defernot_asyncnot(self): self.assertTrue("defer=" not in rendered) def test_bundle_defer_asyncnot(self): - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.load_async = False bundle.load_defer = True view = ScriptsView(self.app, self.app.REQUEST, None, None) @@ -52,7 +55,7 @@ def test_bundle_defer_asyncnot(self): self.assertTrue("defer=" in rendered) def test_bundle_defernot_async(self): - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.load_async = True bundle.load_defer = False view = ScriptsView(self.app, self.app.REQUEST, None, None) @@ -62,7 +65,7 @@ def test_bundle_defernot_async(self): self.assertTrue("defer=" not in rendered) def test_bundle_defer_async(self): - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.load_async = True bundle.load_defer = True view = ScriptsView(self.app, self.app.REQUEST, None, None) @@ -80,7 +83,7 @@ def test_scripts_viewlet(self): def test_scripts_data_bundle_attr(self): scripts = ScriptsView(self.layer["portal"], self.layer["request"], None) - self._make_test_bundle() + _make_test_bundle() scripts.update() result = scripts.render() self.assertIn('data-bundle="foobar"', result) @@ -111,7 +114,7 @@ def test_disabled_bundle_not_rendered(self): scripts = ScriptsView(self.layer["portal"], subreq, None) # add some bundle to test with - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.enabled = False scripts.update() result = scripts.render() @@ -131,7 +134,7 @@ def test_add_bundle_on_request_with_subrequest(self): scripts = ScriptsView(self.layer["portal"], subreq, None) # add some bundle to test with - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.enabled = False scripts.update() result = scripts.render() @@ -151,22 +154,63 @@ def test_remove_bundle_on_request_with_subrequest(self): scripts = ScriptsView(self.layer["portal"], subreq, None) # add some bundle to test with - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.enabled = True scripts.update() result = scripts.render() self.assertNotIn("http://test.foo/test.css", result) def test_bundle_depends(self): - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.depends = "plone" view = ScriptsView(self.app, self.app.REQUEST, None, None) view.update() results = view.render() self.assertIn("http://foo.bar/foobar.js", results) + def test_js_bundle_depends_all(self): + # Create a test bundle, which has unspecified dependencies and is + # rendered in order as defined. + _make_test_bundle(name="a") + + # Create a test bundle, which depends on "all" other and thus rendered + # last. + _make_test_bundle(name="last", depends="all") + + # Create a test bundle, which has unspecified dependencies and is + # rendered in order as defined. + _make_test_bundle(name="b") + + view = ScriptsView(self.layer["portal"], self.layer["request"], None) + view.update() + results = view.render() + + parser = etree.HTMLParser() + parsed = etree.fromstring(results, parser) + scripts = parsed.xpath("//script") + + # The last element is our JS, depending on "all". + self.assertEqual( + "http://foo.bar/last.js", + scripts[-1].attrib["src"], + ) + + # The first resource is our JS, which was defined with unspecified + # dependency first. + self.assertEqual( + "http://foo.bar/a.js", + scripts[0].attrib["src"], + ) + + # The second resource is our JS, which was defined with unspecified + # dependency last. + self.assertEqual( + "http://foo.bar/b.js", + scripts[1].attrib["src"], + ) + def test_bundle_depends_on_multiple(self): - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.depends = "plone,eventedit" view = ScriptsView(self.app, self.app.REQUEST, None, None) view.update() @@ -174,7 +218,7 @@ def test_bundle_depends_on_multiple(self): self.assertIn("http://foo.bar/foobar.js", results) def test_bundle_depends_on_missing(self): - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.depends = "nonexistsinbundle" view = ScriptsView(self.app, self.app.REQUEST, None, None) view.update() @@ -208,7 +252,7 @@ def test_resource_bogus(self): ) def test_relative_uri_resource(self): - bundle = self._make_test_bundle() + bundle = _make_test_bundle() bundle.jscompilation = "//foo.bar/foobar.js" view = ScriptsView(self.app, self.app.REQUEST, None, None) view.update() @@ -217,6 +261,19 @@ def test_relative_uri_resource(self): class TestStylesViewlet(PloneTestCase.PloneTestCase): + def _make_test_bundle(self): + registry = getUtility(IRegistry) + + bundles = registry.collectionOfInterface( + IBundleRegistry, prefix="plone.bundles" + ) + bundle = bundles.add("foobar") + bundle.name = "foobar" + bundle.jscompilation = "http://foo.bar/foobar.js" + bundle.csscompilation = "http://foo.bar/foobar.css" + bundle.resources = ["foobar"] + return bundle + def test_styles_viewlet(self): styles = StylesView(self.layer["portal"], self.layer["request"], None) styles.update() @@ -323,6 +380,86 @@ def test_remove_bundle_on_request_with_subrequest(self): result = scripts.render() self.assertNotIn("http://test.foo/test.min.js", result) + def test_css_bundle_depends_all(self): + # Create a test bundle, which has unspecified dependencies and is + # rendered in order as defined. + _make_test_bundle(name="a") + + # Create a test bundle, which depends on "all" other and thus rendered + # last. + _make_test_bundle(name="last", depends="all") + + # Create a test bundle, which has unspecified dependencies and is + # rendered in order as defined. + _make_test_bundle(name="b") + + view = StylesView(self.layer["portal"], self.layer["request"], None) + view.update() + results = view.render() + + parser = etree.HTMLParser() + parsed = etree.fromstring(results, parser) + styles = parsed.xpath("//link") + + # The last element is our CSS, depending on "all". + self.assertEqual( + "http://foo.bar/last.css", + styles[-1].attrib["href"], + ) + + # The second last element is the theme barceloneta theme CSS. + self.assertTrue( + "++theme++barceloneta/css/barceloneta.min.css" in styles[-2].attrib["href"], + ) + + # The first resource is our CSS, which was defined with unspecified + # dependency. + self.assertEqual( + "http://foo.bar/a.css", + styles[0].attrib["href"], + ) + + # The second resource is our CSS, which was defined with unspecified + # dependency first. + self.assertEqual( + "http://foo.bar/b.css", + styles[1].attrib["href"], + ) + + def test_css_bundle_depends_all_but_custom(self): + registry = getUtility(IRegistry) + + custom_key = "plone.app.theming.interfaces.IThemeSettings.custom_css" + registry[custom_key] = "html { background-color: red; }" + + # Create a test bundle, which depends on "all" other and thus rendered + # after all except the custom styles. + _make_test_bundle(name="almost-last", depends="all") + + view = StylesView(self.layer["portal"], self.layer["request"], None) + view.update() + results = view.render() + + parser = etree.HTMLParser() + parsed = etree.fromstring(results, parser) + styles = parsed.xpath("//link") + + # The last element is are the custom styles. + self.assertTrue( + "@@custom.css" in styles[-1].attrib["href"], + ) + + # The second last element is now our CSS, depending on "all". + self.assertEqual( + "http://foo.bar/almost-last.css", + styles[-2].attrib["href"], + ) + + # The third last element is the theme barceloneta theme CSS. + self.assertTrue( + "++theme++barceloneta/css/barceloneta.min.css" in styles[-3].attrib["href"], + ) + class TestExpressions(PloneTestCase.PloneTestCase): def logout(self): diff --git a/news/4074.feature b/news/4074.feature new file mode 100644 index 0000000000..1b41509ed2 --- /dev/null +++ b/news/4074.feature @@ -0,0 +1,17 @@ +Allow bundles to be rendered after all other. + +JS and CSS resources can now be rendered after all other resources in their +resource group including the theme (e.g. the Barceloneta theme CSS). + +There is a exception for custom CSS which can be defined in the theming +controlpanel. This one is always rendered as last style resource. + +To render a resource after all other give it the "depends" value of "all". This +indicates that the resource depends on all other being rendered before. The +resource is then rendered as last resource of it's resource group. + +This allows to override a theme with custom CSS from a bundle instead of having +to add the CSS customizations to the registry via the "custom_css" settings. +As a consequence, theme customization can now be done in the filesystem in +ordinary CSS files instead of being bound to a time consuming workflow which +involces upgrading the custom_css registry after every change.