node_type_form_submit() uses sloppy str_replace
hunmonk - October 5, 2007 - 21:58
| Project: | Drupal |
| Version: | 5.x-dev |
| Component: | node system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | hunmonk |
| Status: | patch (to be ported) |
Description
the str_replace() used to perform the deletion of old variables when a node type is edited is sloppy -- if the variable name itself contains the node type name anywhere in the string, the variable name will be misinterpreted, and the variable will not be deleted. attached fixes this up to be specific.
| Attachment | Size |
|---|---|
| node_type_variable_handling.patch | 1.06 KB |
#1
can you provide steps how to test this patch, please
#2
basically, you need to go to the node type edit form for a node type (ex. 'page' node type at admin/content/types/page), change the 'Type' setting to rename the type, save it, and make sure that that persistent variables are properly transferred to the new names (ie, if you change from 'page' to 'blah' for the type name, then variable 'foo_blah' should be created w/ the same value as 'foo_page', then 'foo_page' should be deleted.
#3
ok, now I understand the problem with the str_replace function
created a new patch, which is much more readable, I think
how can I test the 'Reset to defaults' option? I changed it to delete the old variable, because that would the correct variable to delete. But the two variable names should be the same in this case anyway.
#4
@Pasqualle: couple of things:
Code style looks good. I fully tested the lastest patch, and it works properly if the type name changes, doesn't change, or if variables are reset to defaults.
The only thing I might suggest is to write out $variable_new, instead of $var_new -- but it's probably fine as is.
#5
replaced
$var_new -> $variable_new
$var_old -> $variable_old
#6
notice: Undefined index: type in ..\modules\node\node.module on line 569.
I see a php notice, when testing the "reset to defaults" function, but it was not introduced with this patch. So creating a new issue for it.
the "reset to defaults" and the type change is working correctly, so RTBC
#7
This is an obvious programmer mistake. Explanation:
before patch
<?php
// Save or reset persistent variable values.
foreach ($variables as $key => $value) {
$key .= '_'. $type->type;
if ($op == t('Reset to defaults')) {
variable_del($key);
}
else {
if (is_array($value)) {
$value = array_keys(array_filter($value));
}
variable_set($key, $value);
if ($type->old_type != $type->type) {
$key = str_replace($type->type, $type->old_type, $key);
variable_del($key);
}
}
}
?>
after patch
<?php
// Save or reset persistent variable values.
foreach ($variables as $key => $value) {
$variable_new = $key .'_'. $type->type;
$variable_old = $key .'_'. $type->old_type;
if ($op == t('Reset to defaults')) {
variable_del($variable_old);
}
else {
if (is_array($value)) {
$value = array_keys(array_filter($value));
}
variable_set($variable_new, $value);
if ($variable_new != $variable_old) {
variable_del($variable_old);
}
}
}
?>
try
$key = 'comment'$type->type = 'comment'
$type->old_type = 'foo'
after str_replace $key == 'foo_foo' but should be 'comment_foo'
#8
Thanks, committed.
#9
Shouldn't this be backported ?