Posted by bcmiller0 on January 27, 2012 at 8:04pm
4 followers
| Project: | Select (or other) |
| Version: | 6.x-2.8 |
| Component: | Miscellaneous |
| Category: | bug report |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Undefined index: op, happens as a notice on 'drush features' with select_or_other module installed. As it assumes $_POST['op'] is set.
Comments
#1
Here is a patch.
#2
I'm not convinced you know what you're doing with that patch there. I suspect you've done *just* enough to make the error go away, but you have changed the logic of the if statement completely. Basically now it says "if $_POST isn't set, don't execute the code at all." which I don't think was the intention there.
#3
I did want that as figured if there wasn't a POST then this code should not happen. I could be wrong but this has fixed our notice and haven't noticed any adverse effects as of yet.
#4
OK if you intended that then that's fine :P Just pointing out that if errors were suppressed this would change the behavior.
#5
I still think that function is used in situations where we can't use the new code.
It's a dodgy hook function #780566: can't use hook_content_allowed_values_alter() everywhere
I'm thinking of changing it to this:
<?phpif (
$field['widget']['module'] == 'select_or_other' &&
(empty($_POST['op']) || ( !empty($_POST['op']) && $_POST['op'] != t('Save'))) &&
(empty($_POST['op']) || ( !empty($_POST['op']) && $_POST['op'] != t('Delete'))) &&
) {
?>
#6
Not sure what you mean here. IMO suppressing errors is almost never the correct solution.
#7
No, what I mean is that since most Drupal 6 websites are configured to not display PHP notices, the fix does not actually improve anything for most installations, but does change the workflow/logic which could have unwanted side effects.
I hope the code I posted above can remove the notice from those sites where PHP notices are turned on, as well as not change the logic of the if-statement.
It's silly to worry too much about because the whole if-statement should ideally not exist at all - it's a bad hack. The code kinda assumes if $_POST['op'] is present then we have just submitted a node add/edit page - which is an unreliable assumption, because that hook can be invoked in other cases, and $_POST['op'] can be present for other reasons.
#8
Ahh, thanks. And thanks for the code snip, we'll put that in a patch. We run with E_NOTICES on in development environments, so we usually have to patch these things.
#9
Thanks, here is a patch as suggested by danielb, which i agree now is the better way to go with this.
#10
committed, thanks
#11
that version 9 had extra && in thee hope you got the version -10 one here it is if not.
#12
Automatically closed -- issue fixed for 2 weeks with no activity.