Merge pull request #42 from plone/mamico-imagescaling

Fix imagescaling
plone · Mar 15, 2021 · b6bb45c · b6bb45c
2 parents 2609ece + 53e7769
commit b6bb45c
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 11 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -4,7 +4,12 @@ Changelog
 3.2.2 (unreleased)
 ------------------
 
-- Nothing changed yet.
+Bug fixes:
+
+- Fix imagescaling: missing srcset initialization, and
+  wrong scale arguments (according to latest p.namedfile).
+  Add test for imagescaling.
+  [mamico]
 
 
 3.2.1 (2020-09-26)

diff --git a/plone/app/tiles/demo.py b/plone/app/tiles/demo.py
@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+from plone.namedfile.field import NamedImage
 from plone import tiles
 from plone.supermodel.model import fieldset
 from zope import schema
@@ -19,11 +20,21 @@ class IPersistentTileData(Interface):
 
     message = schema.TextLine(title=u"Persisted message")
     counter = schema.Int(title=u"Counter")
+    image = NamedImage(title=u"Image", required=False)
 
     fieldset('counter', label=u"Counter", fields=['counter'])
 
 
 class PersistentTile(tiles.PersistentTile):
+    __name__ = 'plone.app.tiles.demo.persistent'
+
+    def Title(self):
+        return self.data['message']
+
+    @property
+    def image(self):
+        return self.data['image']
+
     def __call__(self):
         return "<html><body><b>Persistent tile %s #%d</b></body></html>" % (
             self.data['message'],

diff --git a/plone/app/tiles/imagescaling.py b/plone/app/tiles/imagescaling.py
@@ -64,6 +64,7 @@ def __init__(self, context, request, **info):
             name = info['fieldname']
         self.__name__ = '%s.%s' % (name, extension)
         self.url = '%s/@@images/%s' % (url, self.__name__)
+        self.srcset = info.get("srcset", [])
 
     def index_html(self):
         """ download the image """
@@ -149,7 +150,7 @@ def modified(self):
                 mtime += v._p_mtime
         return mtime
 
-    def scale(self, fieldname=None, scale=None, height=None, width=None, **parameters):
+    def scale(self, fieldname=None, scale=None, height=None, width=None, direction="thumbnail", **parameters):
         if fieldname is None:
             fieldname = IPrimaryFieldInfo(self.context).fieldname
         if scale is not None:
@@ -159,7 +160,7 @@ def scale(self, fieldname=None, scale=None, height=None, width=None, **parameter
             width, height = available[scale]
         storage = AnnotationStorage(self.context, self.modified)
         info = storage.scale(
-            factory=self.create, fieldname=fieldname, height=height, width=width, **parameters
+            factory=self.create, fieldname=fieldname, height=height, width=width, direction=direction, **parameters
         )
         if info is not None:
             info['fieldname'] = fieldname

diff --git a/plone/app/tiles/tests/test_forms.py b/plone/app/tiles/tests/test_forms.py
@@ -90,18 +90,19 @@ def set(self, value):
             self.browser.getControl(name=name).value = str(counter)
             self.browser.getControl(label='Save').click()
 
-            # Set should have been called twice, once for each field
-            self.assertEqual(SetCountingDataManager.set_called, 2)
+            # Set should have been called three times, once for each field
+            self.assertEqual(SetCountingDataManager.set_called, 3)
 
+            SetCountingDataManager.set_called = 0
             url = self.browser.url.replace('@@', '@@edit-tile/')
             self.browser.open(url)
             name = 'plone.app.tiles.demo.persistent.message'
             self.browser.getControl(name=name).value = 'blah'
             self.browser.getControl(label='Save').click()
 
-            # Should have been called three times now,
+            # Should have been called twice now,
             # because the counter field has not changed.
-            self.assertEqual(SetCountingDataManager.set_called, 3)
+            self.assertEqual(SetCountingDataManager.set_called, 2)
         finally:
             # Make sure our useless data manager gets deregistered so it
             # doesn't break everything
@@ -125,7 +126,7 @@ def test_edit_form_save_data(self):
         # Check the default.
         content = form.getContent()
         # Comparing two dicts is apparently not done in Python 3.  So we split.
-        self.assertEqual(list(content.keys()), ["message", "counter"])
+        self.assertEqual(list(content.keys()), ["message", "counter", "image"])
         self.assertIsNone(content["message"])
         self.assertIsNone(content["counter"])
 
@@ -136,7 +137,7 @@ def test_edit_form_save_data(self):
         form.handleSave(form=form, action=None)
         content = form.getContent()
         # Comparing two dicts is apparently not done in Python 3.  So we split.
-        self.assertEqual(list(content.keys()), ["message", "counter"])
+        self.assertEqual(list(content.keys()), ["message", "counter", "image"])
         self.assertEqual(content["message"], "hello")
         self.assertEqual(content["counter"], 1)
 
@@ -148,7 +149,7 @@ def test_edit_form_save_data(self):
         form.set_dummy_data({"message": NOT_CHANGED, "counter": 2})
         form.handleSave(form=form, action=None)
         content = form.getContent()
-        self.assertEqual(list(content.keys()), ["message", "counter"])
+        self.assertEqual(list(content.keys()), ["message", "counter", "image"])
         self.assertEqual(content["message"], "hello")
         self.assertEqual(content["counter"], 2)
 
@@ -157,6 +158,6 @@ def test_edit_form_save_data(self):
         form.set_dummy_data({"message": "bye"})
         form.handleSave(form=form, action=None)
         content = form.getContent()
-        self.assertEqual(list(content.keys()), ["message", "counter"])
+        self.assertEqual(list(content.keys()), ["message", "counter", "image"])
         self.assertEqual(content["message"], "bye")
         self.assertEqual(content["counter"], 2)
diff --git a/plone/app/tiles/tests/test_imagescaling.py b/plone/app/tiles/tests/test_imagescaling.py
@@ -0,0 +1,59 @@
+# -*- coding: utf-8 -*-
+import six
+import unittest
+
+from DateTime import DateTime
+from plone.app.testing import TEST_USER_ID
+from plone.app.testing import setRoles
+from plone.app.tiles.demo import PersistentTile
+from plone.app.tiles.testing import PLONE_APP_TILES_INTEGRATION_TESTING
+from plone.namedfile.file import NamedImage
+from plone.namedfile.tests import getFile
+from plone.tiles.interfaces import ITileDataManager
+
+
+class MockNamedImage(NamedImage):
+    _p_mtime = DateTime().millis()
+
+
+class TestImageScaling(unittest.TestCase):
+
+    layer = PLONE_APP_TILES_INTEGRATION_TESTING
+
+    def setUp(self):
+        self.portal = self.layer['portal']
+        self.request = self.layer['request']
+        # we need to have the Manager role to be able to add things
+        # to the portal root
+        setRoles(self.portal, TEST_USER_ID, ['Manager',])
+
+    def testImageScaling(self):
+        tile = PersistentTile(self.portal, self.request)
+        tile.id = 'mytile'
+        data = getFile('image.png')
+        data = {
+            'message': u'foo',
+            'image': MockNamedImage(data, 'image/png', u'image.png'),
+        }
+        dm = ITileDataManager(tile)
+        dm.set(data)
+
+        images = self.portal.restrictedTraverse('@@plone.app.tiles.demo.persistent/mytile/@@images')
+        if six.PY2:
+            assertRegex = self.assertRegexpMatches
+        else:
+            assertRegex = self.assertRegex
+
+        assertRegex(
+            images.tag('image', width=10),
+            r'<img src="http://nohost/plone/@@plone.app.tiles.demo.persistent/mytile/@@images/[a-z0-9-]+\.png" '
+            r'alt="foo" title="foo" height="10" width="10" />'
+        )
+
+        scale = images.scale('image', scale='mini')
+        self.assertEqual(scale.data.data[:4], b'\x89PNG')
+        self.assertEqual(scale.data.getImageSize(), (200, 200))
+        assertRegex(
+            scale.url,
+            r'http://nohost/plone/@@plone.app.tiles.demo.persistent/mytile/@@images/[a-z0-9-]+\.png'
+        )