Restore "attrs" template variable support (#861)

* fix lost "attrs" variable

* provide `CHANGES.rst` entries

* fix `isort` problem

* fix typo

* tentatively remove workarounds for `malthe/chameleon#323

* Revert "tentatively remove workarounds for `malthe/chameleon#323"

This reverts commit 5f49275.

At least for the moment, `chameleon` is not yet ready to drop the work around.
In addition, our test suite is not sufficiently complete to verify the safety
of all `chameleon` features without the work around in place.

* clearly state in the comments related to `chameleon` work arounds that those might go away with ``chameleon > 3.8.0``;
add a not to the Zope Book that ``chameleon`` implements `attrs` and `default` in a special way and that their value should not be changed

* add test for `CONTEXTS`; restore `CONTEXTS` related documentation (improved)

Co-authored-by: Jens Vagelpohl <jens@netz.ooo>

CHANGES.rst

@@ -13,6 +13,12 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
 
 - Add ``tal:switch`` test
 
+- Support the ``attrs`` predefined template variable again (as
+  far as ``chameleon`` allows it)
+  (`#860 <https://github.com/zopefoundation/Zope/issues/860>`_).
+
+- Improve documentation of ``CONTEXTS`` in the "Zope Book".
+
 - Drop support for Python 3.5 as it will run out of support soon.
   (`#841 <https://github.com/zopefoundation/Zope/issues/841>`_)
 

docs/zopebook/AppendixC.rst

@@ -674,10 +674,6 @@ These are the names always available to TALES expressions in Zope:
 - *attrs*- - a dictionary containing the initial values of the attributes of
   the current statement tag.
 
-- *CONTEXTS*- - the list of standard names (this list). This can be used to
-  access a built-in variable that has been hidden by a local or global variable
-  with the same name.
-
 - *root*- - the system's top-most object: the Zope root folder.
 
 - *context*- - the object to which the template is being applied.
@@ -698,6 +694,18 @@ Note the names `root`, `context`, `container`, `template`, `request`, `user`, an
 `modules` are optional names supported by Zope, but are not required by the
 TALES standard.
 
+Note that the (popular) ``chameleon`` template engine implements ``attrs``
+and ``default`` not as standard variables but in a special way.
+Trying to change their value may have undefined effects.
+
+Note that beside variables you can use ``CONTEXTS``
+as initial element in a path expression. Its value is a mapping
+from predefined variable names to their value. This can be used to
+access the predefined variable when it is hidden by a user defined
+definition for its name. Again, `attrs` is special; it is not covered
+by `CONTEXTS`.
+
+
 TALES Exists expressions
 ========================
 

src/Products/PageTemplates/engine.py

@@ -147,9 +147,10 @@ def _compile_zt_expr(type, expression, engine=None, econtext=None):
 
 class _C2ZContextWrapper(Context):
     """Behaves like "zope" context with vars from "chameleon" context."""
-    def __init__(self, c_context):
+    def __init__(self, c_context, attrs):
         self.__c_context = c_context
         self.__z_context = c_context["__zt_context__"]
+        self.__attrs = attrs
 
     # delegate to ``__c_context``
     @property
@@ -160,6 +161,8 @@ def __getitem__(self, key):
         try:
             return self.__c_context.__getitem__(key)
         except NameError:  # Exception for missing key
+            if key == "attrs":
+                return self.__attrs
             raise KeyError(key)
 
     def getValue(self, name, default=None):
@@ -212,11 +215,7 @@ def __getattr__(self, attr):
         return getattr(self.__z_context, attr)
 
 
-def _c_context_2_z_context(c_context):
-    return _C2ZContextWrapper(c_context)
-
-
-_c_context_2_z_context_node = Static(Symbol(_c_context_2_z_context))
+_c_context_2_z_context_node = Static(Symbol(_C2ZContextWrapper))
 
 
 class MappedExpr:
@@ -260,14 +259,24 @@ def __init__(self, type, expression, zt_engine):
                                     raise ExpressionError("$ unsupported", spe)
 
     def __call__(self, target, c_engine):
+        # The convoluted handling of ``attrs`` below was necessary
+        # for some ``chameleon`` versions to work around
+        # "https://github.com/malthe/chameleon/issues/323".
+        # The work round is partial only: until the ``chameleon``
+        # problem is fixed, `attrs` cannot be used inside ``tal:define``.
+        # Potentially, ``attrs`` handling could be simplified
+        # for ``chameleon > 3.8.0``.
         return template(
+            "try: __zt_tmp = attrs\n"
+            "except NameError: __zt_tmp = None\n"
             "target = compile_zt_expr(type, expression, econtext=econtext)"
-            "(c2z_context(econtext))",
+            "(c2z_context(econtext, __zt_tmp))",
             target=target,
             compile_zt_expr=_compile_zt_expr_node,
             type=ast.Str(self.type),
             expression=ast.Str(self.expression),
-            c2z_context=_c_context_2_z_context_node)
+            c2z_context=_c_context_2_z_context_node,
+            attrs=ast.Name("attrs", ast.Load()))
 
 
 class MappedExprType:

src/Products/PageTemplates/tests/testC2ZContext.py

@@ -23,7 +23,7 @@ class C2ZContextTests(unittest.TestCase):
     def setUp(self):
         self.c_context = c_context = Scope()
         c_context["__zt_context__"] = Context(None, {})
-        self.z_context = _C2ZContextWrapper(c_context)
+        self.z_context = _C2ZContextWrapper(c_context, None)
 
     def test_elementary_functions(self):
         c = self.z_context
@@ -51,7 +51,7 @@ def test_elementary_functions(self):
     def test_setGlobal(self):
         top_context = self.z_context
         c_context = self.c_context.copy()  # context push
-        c = _C2ZContextWrapper(c_context)  # local ``zope`` context
+        c = _C2ZContextWrapper(c_context, None)  # local ``zope`` context
         c.setLocal("a", "A")
         self.assertIn("a", c)
         self.assertNotIn("a", top_context)
@@ -77,3 +77,9 @@ def test_unimplemented(self):
     def test_attribute_delegation(self):
         c = self.z_context
         self.assertIsNone(c._engine)
+
+    def test_attrs(self):
+        c = self.z_context
+        self.assertIsNone(c["attrs"])
+        c.setLocal("attrs", "hallo")
+        self.assertEqual(c["attrs"], "hallo")

src/Products/PageTemplates/tests/testVariables.py

@@ -0,0 +1,142 @@
+from unittest import TestCase
+
+from OFS.Folder import Folder
+from zope.component.testing import PlacelessSetup
+
+from ..Expressions import getTrustedEngine
+from ..ZopePageTemplate import ZopePageTemplate
+from .util import useChameleonEngine
+
+
+class PageTemplate(ZopePageTemplate):
+    def pt_getEngine(self):
+        return getTrustedEngine()
+
+
+class TestPredefinedVariables(PlacelessSetup, TestCase):
+    """test predefined variables
+
+    as documented by
+    `<https://zope.readthedocs.io/en/latest/zopebook/AppendixC.html#built-in-names`_
+    """
+
+    # variables documented in the Zope Book
+    VARIABLES = set((
+        "nothing",
+        "default",
+        "options",
+        "repeat",
+        # "attrs",  # special -- not contained in ``CONTEXTS``
+        "root",
+        "context",
+        "container",
+        "template",
+        "request",
+        "user",
+        "modules",
+        #   special
+        #     - only usable as initial component in path expr
+        #     - not contained in ``CONTEXTS``
+        # "CONTEXTS",
+        ))  # noqa: E123
+
+    def setUp(self):
+        super(TestPredefinedVariables, self).setUp()
+
+        def add(dest, id, factory):
+            nid = dest._setObject(id, factory(id))
+            obj = dest._getOb(nid)
+            if hasattr(obj, "_setId"):
+                obj._setId(nid)
+            return obj
+
+        self.root = Folder()
+        self.root.getPhysicalRoot = lambda: self.root
+        self.f = add(self.root, "f", Folder)
+        self.g = add(self.f, "g", Folder)
+        self.template = add(self.f, "t", PageTemplate)
+        # useChameleonEngine()
+
+    def test_nothing(self):
+        self.assertIsNone(self.check("nothing"))
+
+    def test_default(self):
+        self.assertTrue(self.check("default"))
+
+    def test_options(self):
+        self.assertTrue(self.check("options"))
+
+    def test_repeat(self):
+        self.check("repeat")
+
+    def test_attrs(self):
+        attrs = self.check("attrs")
+        self.assertEqual(attrs["attr"], "attr")
+
+    # ``test_attrs`` should have the previous definition
+    # Unfortunately, "https://github.com/malthe/chameleon/issues/323"
+    # (``attrs`` cannot be used in ``tal:define``)
+    # lets it fail.
+    # We must therefore test (what works with the current
+    # ``chameleon``) in a different way.
+    # Note ``chameleon > 3.8.0`` likely will allow the previous definition
+    def test_attrs(self):  # noqa: F811
+        t = self.g.t
+        t.write("<div attr='attr' tal:content='python: attrs[\"attr\"]'/>")
+        # the two template engines use different quotes - we
+        #  must normalize
+        self.assertEqual(t().replace("'", '"'), '<div attr="attr">attr</div>')
+
+    def test_root(self):
+        self.assertEqual(self.check("root"), self.root)
+
+    def test_context(self):
+        self.assertEqual(self.check("context"), self.g)
+
+    def test_container(self):
+        self.assertEqual(self.check("container"), self.f)
+
+    def test_template(self):
+        self.assertEqual(self.check("template"), self.template)
+
+    def test_request(self):
+        self.check("request")
+
+    def test_user(self):
+        self.check("user")
+
+    def test_modules(self):
+        self.check("modules")
+
+    def test_CONTEXTS(self):
+        # the "Zope Book" describes ``CONTEXTS`` as a variable.
+        # But, in fact, it is not a (regular) variable but a special
+        # case of the initial component of a (sub)path expression.
+        # As a consequence, it cannot be used in a Python expression
+        # but only as the first component in a path expression
+        # Therefore, we cannot use ``check`` to access it.
+        t = self.g.t
+        t.write("""<div tal:define="
+                x CONTEXTS;
+                dummy python:options['acc'].append(x)" />""")
+        acc = []
+        t(acc=acc)
+        ctx = acc[0]
+        self.assertIsInstance(ctx, dict)
+        # all variables included?
+        self.assertEqual(len(self.VARIABLES - set(ctx)), 0)
+
+    def check(self, what):
+        t = self.g.t
+        t.write("<div attr='attr' "
+                """tal:define="dummy python:options['acc'].append(%s)"/>"""
+                % what)
+        acc = []
+        t(acc=acc)
+        return acc[0]
+
+
+class TestPredefinedVariables_chameleon(TestPredefinedVariables):
+    def setUp(self):
+        super(TestPredefinedVariables_chameleon, self).setUp()
+        useChameleonEngine()