- Fix broken title_and_id behavior (fixes #574)

CHANGES.rst

@@ -14,6 +14,9 @@ https://github.com/zopefoundation/Zope/blob/4.0a6/CHANGES.rst
 Fixes
 +++++
 
+- Fix broken ``title_and_id`` behavior
+  (`#574 <https://github.com/zopefoundation/Zope/issues/574>`_)
+
 - Fix broken ZMI DTML rendering for mixed unicode/bytes content
   (`#271 <https://github.com/zopefoundation/Zope/issues/271>`_)
 

src/OFS/SimpleItem.py

@@ -24,6 +24,7 @@
 import sys
 import time
 
+import six
 from six import reraise
 
 from AccessControl.class_init import InitializeClass
@@ -62,6 +63,7 @@
 from zExceptions import Redirect
 from zExceptions.ExceptionFormatter import format_exception
 from zope.interface import implementer
+from ZPublisher.HTTPRequest import default_encoding
 
 
 if bbb.HAS_ZSERVER:
@@ -188,6 +190,20 @@ def title_and_id(self):
         if callable(title):
             title = title()
         id = self.getId()
+        # Make sure we don't blindly concatenate encoded and unencoded data
+        # This may happen under Python 2 where the id is encoded but the
+        # title is unencoded.
+        if title and type(id) is not type(title):
+            if six.PY2:
+                if isinstance(id, six.text_type):
+                    id = id.encode(default_encoding)
+                if isinstance(title, six.text_type):
+                    title = title.encode(default_encoding)
+            else:
+                if isinstance(id, six.binary_type):
+                    id = id.decode(default_encoding)
+                if isinstance(title, six.binary_type):
+                    title = title.decode(default_encoding)
         return title and ("%s (%s)" % (title, id)) or id
 
     def this(self):

src/OFS/tests/testSimpleItem.py

@@ -1,5 +1,7 @@
 import unittest
 
+import six
+
 
 class TestItem(unittest.TestCase):
 
@@ -80,16 +82,56 @@ def test_interfaces(self):
 
 class TestSimpleItem(unittest.TestCase):
 
+    def _getTargetClass(self):
+        from OFS.SimpleItem import SimpleItem
+        return SimpleItem
+
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
+
     def test_interfaces(self):
         from OFS.interfaces import ISimpleItem
-        from OFS.SimpleItem import SimpleItem
         from zope.interface.verify import verifyClass
 
-        verifyClass(ISimpleItem, SimpleItem)
+        verifyClass(ISimpleItem, self._getTargetClass())
+
+    def test_title_or_id_nonascii(self):
+        unencoded_id = u'\xfc\xe4\xee\xe9\xdf_id'
+        unencoded_title = u'\xfc\xe4\xee\xe9\xdf Title'
+        item = self._makeOne()
+
+        item.id = unencoded_id
+        self.assertEqual(item.title_or_id(), unencoded_id)
+
+        item.title = unencoded_title
+        self.assertEqual(item.title_or_id(), unencoded_title)
+
+    def test_title_and_id_nonascii(self):
+        unencoded_id = u'\xfc\xe4\xee\xe9\xdf_id'
+        encoded_id = unencoded_id.encode('UTF-8')
+        unencoded_title = u'\xfc\xe4\xee\xe9\xdf Title'
+        encoded_title = unencoded_title.encode('UTF-8')
+        item = self._makeOne()
+
+        item.id = unencoded_id
+        self.assertEqual(item.title_and_id(), unencoded_id)
+
+        item.title = unencoded_title
+        self.assertIn(unencoded_id, item.title_and_id())
+        self.assertIn(unencoded_title, item.title_and_id())
+
+        # Now mix encoded and unencoded. The combination is a native
+        # string, meaning encoded on Python 2 and unencoded on Python 3
+        item.id = encoded_id
+        if six.PY3:
+            self.assertIn(unencoded_id, item.title_and_id())
+            self.assertIn(unencoded_title, item.title_and_id())
+        else:
+            self.assertIn(encoded_id, item.title_and_id())
+            self.assertIn(encoded_title, item.title_and_id())
 
     def test_standard_error_message_is_called(self):
         from zExceptions import BadRequest
-        from OFS.SimpleItem import SimpleItem
 
         # handle_errors should default to True. It is a flag used for
         # functional doctests. See ZPublisher/Test.py and
@@ -106,7 +148,7 @@ def __call__(self, **kw):
                 self.kw.clear()
                 self.kw.update(kw)
 
-        item = SimpleItem()
+        item = self._makeOne()
         item.standard_error_message = sem = StandardErrorMessage()
 
         try: