Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WrongType saving advanced control panel on Python 2. #183

Closed
mauritsvanrees opened this issue Jun 10, 2020 · 1 comment
Closed

WrongType saving advanced control panel on Python 2. #183

mauritsvanrees opened this issue Jun 10, 2020 · 1 comment
Assignees
@mauritsvanrees
Labels
02 type: regression 11 prio: blocker 22 status: in-progress 31 needs: help

Comments

@mauritsvanrees
Copy link
Member

There are several issues where WrongType is raised on Python 3 when saving the control panel, see #175 and #179. But the same is true on Python 2. This is both with the master branch (where a few things have been fixed for Python 3) and with the last 4.0.6 release.

  • Create a fresh Plone 5.2 site with Python 2.7.
  • Go to the theming control panel, advanced tab.
  • Save it without making any changes.
  • Result:
ERROR   [Zope.SiteErrorLog:252][waitress] 1591821347.640.715833970434
 http://localhost:8080/Plone5/@@theming-controlpanel
Traceback (innermost last):
  Module ZPublisher.WSGIPublisher, line 162, in transaction_pubevents
  Module ZPublisher.WSGIPublisher, line 359, in publish_module
  Module ZPublisher.WSGIPublisher, line 262, in publish
  Module ZPublisher.mapply, line 85, in mapply
  Module ZPublisher.WSGIPublisher, line 63, in call_object
  Module plone.app.theming.browser.controlpanel, line 62, in __call__
  Module plone.app.theming.browser.controlpanel, line 203, in update
  Module plone.registry.recordsproxy, line 60, in __setattr__
  Module plone.registry.registry, line 51, in __setitem__
  Module plone.registry.record, line 82, in _set_value
  Module zope.schema._bootstrapfields, line 291, in validate
  Module zope.schema._bootstrapfields, line 494, in _validate
  Module zope.schema._bootstrapfields, line 337, in _validate
WrongType: ('/++theme++barceloneta/rules.xml', <type 'unicode'>, 'value')

Wait a minute: in Plone 5.2.1 we used version 4.0.4. With that version (and current coredev 5.2) it works.
Update to 4.0.5 and... boom.
What that version changes, is from (my own) PR #172. We no longer call processInputs because as far as we knew at that time: "It is not needed since Zope 4, and not existing in Zope 5." When you call processInputs you see this DeprecationWarning:

processInputs() is deprecated and will be removed in Zope 5.0.
If your view implements IBrowserPage, similar processing is now executed automatically.

Our control panel view does not implement IBrowserPage:

(Pdb++) from zope.publisher.interfaces.browser import IBrowserPage
(Pdb++) self
<Products.Five.browser.metaconfigure.SimpleViewClass from /Users/maurits/community/plone-coredev/5.2/src/plone.app.theming/src/plone/app/theming/browser/controlpanel.pt object at 0x11077a890>
(Pdb++) IBrowserPage.implementedBy(self)
False
(Pdb++) IBrowserPage.providedBy(self)  # to be sure I don't mix up implement/provide
False

processInputs seems to help in smoothing over the differences between Python 2 and 3.
Calling it is still very much needed apparently.
Before calling processInputs (on Python 2):

(Pdb++) pp self.request.form
{'_authenticator': 'ebfbb7d3b910b6382178f1d00392e07b2d611b0e',
 'absolutePrefix': '/++theme++barceloneta',
 'doctype': '<!DOCTYPE html>',
 'extLinksOpenInNewWindow': False,
 'form.button.AdvancedSave': 'Opslaan',
 'hostnameBlacklist': ['127.0.0.1'],
 'markSpecialLinks': False,
 'parameterExpressions': [],
 'readNetwork': False,
 'rules': '/++theme++barceloneta/rules.xml',
 'themeBase': 'Plone Default',
 'themeEnabled': True}

After calling processInputs:

(Pdb++) pp self.request.form
{'_authenticator': u'ebfbb7d3b910b6382178f1d00392e07b2d611b0e',
 'absolutePrefix': u'/++theme++barceloneta',
 'doctype': u'<!DOCTYPE html>',
 'extLinksOpenInNewWindow': False,
 'form.button.AdvancedSave': u'Opslaan',
 'hostnameBlacklist': [u'127.0.0.1'],
 'markSpecialLinks': False,
 'parameterExpressions': [],
 'readNetwork': False,
 'rules': u'/++theme++barceloneta/rules.xml',
 'themeBase': u'Plone Default',
 'themeEnabled': True}

I am working on a branch where at first I add a test that simply saves the control panel. That already fails currently. Reverting the processInputs change seems to help.

cc @MrTango as he discovered and fixed a few WrongType errors recently. Those fixes may no longer be needed when we revert to using processInputs (when available). And maybe they are harmful then because for example a decode is done twice.

cc @jensens because with me he removed processInput calls in several packages.
@mauritsvanrees mauritsvanrees self-assigned this Jun 10, 2020
mauritsvanrees added a commit that referenced this issue Jun 10, 2020
@mauritsvanrees
Do not make a string from custom_css.
e6d07c7 
The field is SourceText, which is unicode on Python 2, and str on Python 3.
So on Python 2, creating a string of it results in a WrongType error.
See #183
mauritsvanrees added a commit that referenced this issue Jun 10, 2020
@mauritsvanrees
Added changelog entry for #183 fix.
5cb479c
This was referenced Jun 10, 2020
Merged
Merged
Closed
Closed
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Jun 11, 2020
@mauritsvanrees
[fc] Repository: plone.app.theming
d3b8b09 
Branch: refs/heads/master
Date: 2020-06-10T23:05:22+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@64f4fec

Added test for saving the advanced control panel.

This should help surface some `WrongType`  exceptions.

Files changed:
M src/plone/app/theming/tests/test_controlpanel.py
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-06-10T23:07:58+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@865d37b

Revert "Do not call processInputs."

This reverts commit dd4f7fc15d1d7ec890780b1f64ad433efaf4fb9e.

Files changed:
M src/plone/app/theming/browser/controlpanel.py
M src/plone/app/theming/browser/mapper.py
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-06-10T23:17:02+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@12e382f

Import processInputs conditionally.

Files changed:
M src/plone/app/theming/browser/controlpanel.py
M src/plone/app/theming/browser/mapper.py
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-06-10T23:21:40+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@e6d07c7

Do not make a string from custom_css.

The field is SourceText, which is unicode on Python 2, and str on Python 3.
So on Python 2, creating a string of it results in a WrongType error.
See plone/plone.app.theming#183

Files changed:
M src/plone/app/theming/browser/controlpanel.py
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-06-10T23:28:43+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@9c64d46

Revert "fix hostnameBlacklist (Theming ControlPanel) in Py3"

This reverts commit d6db78dbb930837c296aae87cea16bacde391a9b.

No longer needed after calling processInputs.

Files changed:
M src/plone/app/theming/browser/controlpanel.pt
M src/plone/app/theming/browser/controlpanel.py
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-06-10T23:38:32+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@5cb479c

Added changelog entry for #183 fix.

Files changed:
A news/183.bugfix
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-06-11T10:11:25+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@a360731

Added upgrade step to reload the theming registry.

Otherwise when you upgrade from Plone 5.2.1, and try to save the theming control panel, you get an error:

```
Traceback (innermost last):
  Module ZPublisher.WSGIPublisher, line 162, in transaction_pubevents
  Module ZPublisher.WSGIPublisher, line 359, in publish_module
  Module ZPublisher.WSGIPublisher, line 262, in publish
  Module ZPublisher.mapply, line 85, in mapply
  Module ZPublisher.WSGIPublisher, line 63, in call_object
  Module plone.app.theming.browser.controlpanel, line 65, in __call__
  Module plone.app.theming.browser.controlpanel, line 215, in update
  Module plone.registry.recordsproxy, line 59, in __setattr__
AttributeError: custom_css_timestamp
```

Files changed:
M src/plone/app/theming/configure.zcml
M src/plone/app/theming/profiles/default/metadata.xml
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-06-11T20:31:42+02:00
Author: Maurits van Rees (mauritsvanrees) <m.van.rees@zestsoftware.nl>
Commit: plone/plone.app.theming@fdbb576

Merge pull request #184 from plone/maurits/issue-183-save-advanced-controlpanel-wrongtype

Call processInputs when available, fixing WrongType

Files changed:
A news/183.bugfix
M src/plone/app/theming/browser/controlpanel.pt
M src/plone/app/theming/browser/controlpanel.py
M src/plone/app/theming/browser/mapper.py
M src/plone/app/theming/configure.zcml
M src/plone/app/theming/profiles/default/metadata.xml
M src/plone/app/theming/tests/test_controlpanel.py
mauritsvanrees added a commit that referenced this issue Sep 23, 2020
@mauritsvanrees
Fixed WrongContainedType for hostnameBlackList on Zope 5.
88d90a4 
See also #183.

This reverts commit 9c64d46
which was itself a revert of "fix hostnameBlacklist (Theming ControlPanel) in Py3"

In Plone 6 (with Zope 5) this is needed again, because `processInputs` no longer exists.
@mauritsvanrees mauritsvanrees mentioned this issue Sep 23, 2020
Merged
@mauritsvanrees
Copy link
Member Author

Broken again on Plone 6 with Zope 5.
PR: #183

mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Sep 24, 2020
@jensens
[fc] Repository: plone.app.theming
6469f37 
Branch: refs/heads/master
Date: 2020-09-23T23:58:51+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@88d90a4

Fixed WrongContainedType for hostnameBlackList on Zope 5.

See also plone/plone.app.theming#183.

This reverts commit 9c64d46bf54ebf5259a8ccf400d329991a2cb70a
which was itself a revert of "fix hostnameBlacklist (Theming ControlPanel) in Py3"

In Plone 6 (with Zope 5) this is needed again, because `processInputs` no longer exists.

Files changed:
A news/183.bugfix
M src/plone/app/theming/browser/controlpanel.pt
M src/plone/app/theming/browser/controlpanel.py
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-09-24T00:06:17+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@bbed30e

Call safe_nativestring on hostnameBlacklist.

That should work on Py 2 and 3 and Plone 5.2 and 6.0.

Files changed:
M src/plone/app/theming/browser/controlpanel.py
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-09-24T00:09:26+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/plone.app.theming@8a67c21

hostnameBlacklist: PY2 needs no changes.

Files changed:
M src/plone/app/theming/browser/controlpanel.py
Repository: plone.app.theming

Branch: refs/heads/master
Date: 2020-09-24T09:45:18+02:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/plone.app.theming@a784c2f

Merge pull request #192 from plone/zope5

Fixed WrongContainedType for hostnameBlackList on Zope 5.

Files changed:
A news/183.bugfix
M src/plone/app/theming/browser/controlpanel.pt
M src/plone/app/theming/browser/controlpanel.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mauritsvanrees mauritsvanrees

Labels
02 type: regression 11 prio: blocker 22 status: in-progress 31 needs: help
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@mauritsvanrees