-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
[bug] Sometimes, on form edits, the correct prior value is not selected #259
Comments
I realize this is because of whitespace, not periods. When values have double spaces in them, these get stripped at somepoint, probably by html-sanitize. |
Is the right answer to this REALLY to strip excess whitespace from the form config? I think we should be asking why we are sanitizing data on writes instead of on reads... the only advantage is that it ensures programmatic users don't inadvertently pull malicious data... but with all the disadvantages of control (we can't pull the original data if we want to, if we sanitize on write). I think at this stage we'll keep it. The change would not be a huge boon at this stage, and it would not solve this issue. I wrote the following quick, recursive excess space remover.... but again it may result in complaints from users who want those extra spaces... def clean_extra_spaces(data, exclude_fields=['description', 'field_label']):
if isinstance(data, str):
# Replace multiple spaces with a single space
return re.sub(r'\s+', ' ', data).strip()
elif isinstance(data, list):
return [clean_extra_spaces(item) for item in data]
elif isinstance(data, dict):
return {
key: clean_extra_spaces(value)
if key not in exclude_fields else value
for key, value in data.items()
}
return data |
An alternative to this would be to use html-sanitize to clean up the form config on write, but that's overkill in most cases. |
Quite frankly, cleaning whitespace on writes was a terrible idea. It forces us to serialize/deserialize the config string ... which overcomplicates things. So, instead, we moved the whitespace cleaning to reads. This may produce unexpected results, which we will monitor and address as they come up. |
Jquery is funky sometimes. I think we need to escape values with periods in them, see https://stackoverflow.com/a/350300.
The text was updated successfully, but these errors were encountered: