[Zope3-checkins] SVN: Zope3/trunk/src/zope/app/form/browser/ Fixed
a bug in SimpleInputWidget. _getFormValue had an evil side
Jim Fulton
jim at zope.com
Thu Jul 21 10:55:00 EDT 2005
Log message for revision 37365:
Fixed a bug in SimpleInputWidget. _getFormValue had an evil side
effect of setting _error. I assumed that this was unintentional, but
there were tests supporting it, which is weird. If client code had
not called getInputValue directly, then there is no reason to set
_error. _getFormValue is only called by widget rendering code, which
should not produce this side effect.
Consider the following use case:
- We present a form
- The form contains widgets that have sub-forms. Here "sub-forms" are
collections of input widgets without a form tag. Many logical
subforms share a common form tag. Processing is dispatched based on
the submit button used in a request.
- A user interacts with a widget by clicking on a sub widget, for
example do do "search and select". They did not click on a submit
button for the overall form.
- The application does not ytry to get input from any of the widgets
nor does it try to validate user input. User input, of any, is know
to be incomplete, because the form-submit buttons haven't been used.
- In response to the users action, the application generates an
updated form by rendering the widgets. Because the user hasn't
tried to submit the form, they should not be presented with error
messages, at least not at the form level.
In summary, widgets should not generate error messages if they haven't
been asked for valid values.
Changed:
U Zope3/trunk/src/zope/app/form/browser/tests/test_itemswidget.py
U Zope3/trunk/src/zope/app/form/browser/tests/test_textwidget.py
U Zope3/trunk/src/zope/app/form/browser/widget.py
-=-
Modified: Zope3/trunk/src/zope/app/form/browser/tests/test_itemswidget.py
===================================================================
--- Zope3/trunk/src/zope/app/form/browser/tests/test_itemswidget.py 2005-07-21 14:51:20 UTC (rev 37364)
+++ Zope3/trunk/src/zope/app/form/browser/tests/test_itemswidget.py 2005-07-21 14:54:59 UTC (rev 37365)
@@ -216,11 +216,13 @@
u'<option value="token2">Two</option>',
u'<option value="token3">Three</option>'])
- def test_error(self):
- widget = self._makeWidget(form={'field.choice': 'ten'})
- widget.setPrefix('field.')
- widget._getFormValue()
- self.assert_(isinstance(widget._error, ConversionError))
+# This test is disabled because it tests for the presense of a missfeature,
+# which has been removed. Did someone actually *want* this?
+## def test_error(self):
+## widget = self._makeWidget(form={'field.choice': 'ten'})
+## widget.setPrefix('field.')
+## widget._getFormValue()
+## self.assert_(isinstance(widget._error, ConversionError))
def test_hidden(self):
widget = self._makeWidget(form={'field.choice': 'token2'})
@@ -413,11 +415,13 @@
'>One</option>\n', 'value="token2"', '>Two</option>\n',
'value="token3"', '>Three</option>', '</select>'])
- def test_error(self):
- widget = self._makeWidget(form={'field.numbers': ['ten']})
- widget.setPrefix('field.')
- widget._getFormValue()
- self.assert_(isinstance(widget._error, ConversionError))
+# This test is disabled because it tests for the presense of a missfeature,
+# which has been removed. Did someone actually *want* this?
+## def test_error(self):
+## widget = self._makeWidget(form={'field.numbers': ['ten']})
+## widget.setPrefix('field.')
+## widget._getFormValue()
+## self.assert_(isinstance(widget._error, ConversionError))
def test_hidden(self):
widget = self._makeWidget(
Modified: Zope3/trunk/src/zope/app/form/browser/tests/test_textwidget.py
===================================================================
--- Zope3/trunk/src/zope/app/form/browser/tests/test_textwidget.py 2005-07-21 14:51:20 UTC (rev 37364)
+++ Zope3/trunk/src/zope/app/form/browser/tests/test_textwidget.py 2005-07-21 14:54:59 UTC (rev 37365)
@@ -253,7 +253,24 @@
value=""
/>
+ """
+def test_no_error_on_render_only():
+ """This is really a test of a bug fix to SimpleInputWidget.
+
+ _error shouldn't be set due to an *internal* call to getInputValue
+ when rendering.
+
+ >>> from zope.publisher.browser import TestRequest
+ >>> from zope.schema import TextLine
+ >>> field = TextLine(__name__='foo')
+ >>> request = TestRequest(form={'field.foo': ''})
+ >>> widget = TextWidget(field, request)
+ >>> ignored = widget()
+ >>> unicode(widget.error())
+ u''
+
+
"""
def test_suite():
Modified: Zope3/trunk/src/zope/app/form/browser/widget.py
===================================================================
--- Zope3/trunk/src/zope/app/form/browser/widget.py 2005-07-21 14:51:20 UTC (rev 37364)
+++ Zope3/trunk/src/zope/app/form/browser/widget.py 2005-07-21 14:54:59 UTC (rev 37365)
@@ -356,9 +356,16 @@
"""Returns a value suitable for use in an HTML form."""
if not self._renderedValueSet():
if self.hasInput():
+
+ # It's insane to use getInputValue this way. It can
+ # cause _error to get set spuriously. We'll work
+ # around this by saving and restoring _error if
+ # necessary.
+ error = self._error
try:
value = self.getInputValue()
except InputErrors:
+ self._error = error
return self.request.form.get(self.name, self._missing)
else:
value = self._getDefault()
More information about the Zope3-Checkins
mailing list