Posted by aldarXose on October 10, 2010 at 12:00pm
8 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | shortcut.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D7 |
Issue Summary
Notice: Undefined index: shortcuts in shortcut_set_customize_submit() (line 314 of X:\dr7\modules\shortcut\shortcut.admin.inc).
Warning: Invalid argument supplied for foreach() in shortcut_set_customize_submit() (line 314 of X:\dr7\modules\shortcut\shortcut.admin.inc).
Comments
#1
I just deleted all the shortcutes from the default set and could not reproduce this error. Care to explain what you did to get the error?
#2
It seems you have to submit the form on the shortcut set configuration page again (after all the shortcuts are gone) in order to see the error.
This was actually reported a while ago at http://drupal.org/node/851204#comment-3191204 but since that issue originally started off about something else, it's better to handle it here.
The attached patch should fix it; it just modifies the form submit handler to make sure the variable exists before using it.
#3
+1 for #2, patch makes the bad thing to go away however the configuration form doesn't makes much sense when it is empty see the attached snapshot. Is there any issue addressing this ?
#4
That's a good point. And actually, if we fix that, we don't need to worry about modifying the submit handler at all, since we'd prevent people from ever being able to submit the form in this situation in the first place.
Let's try the attached patch (and screenshot). This uses the same pattern that other tables in the Drupal admin UI do when they don't have any items in them yet.
#5
Bumping this issue - #1004628: Warning: Invalid argument supplied for foreach() in shortcut_set_customize_submit() was a recently-filed duplicate.
#6
#4: shortcut-empty-set-937380-4.patch queued for re-testing.
#7
Is this RTBC?
#8
Reroll
#9
Great to see this standard being used :)
#10
Looks like this fixes a usability issue, as well as an error. Good stuff.
Committed and pushed to 7.x and 8.x.
#11
Looks good now however I'm not sure how the "Enabled" and "Disabled" row would help. Also the name field in "Add new shortcut" form accepts empty string input perhaps it is already reported in another issue.
#12
@sivaji , see #1271026: Name of shortcut should be required for the empty string input, weird this never has been reported yet.
#13
Actually, the only reason that was introduced is the limit. When that doesn't apply, it should be ok to remove that whole concept.
#14
Automatically closed -- issue fixed for 2 weeks with no activity.