Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
shortcut.module
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
10 Oct 2010 at 12:00 UTC
Updated:
29 Jul 2014 at 19:05 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
IbnDrupal commentedI 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?
Comment #2
David_Rothstein commentedIt 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.
Comment #3
sivaji_ganesh_jojodae commented+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 ?
Comment #4
David_Rothstein commentedThat'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.
Comment #5
David_Rothstein commentedBumping this issue - #1004628: Warning: Invalid argument supplied for foreach() in shortcut_set_customize_submit() was a recently-filed duplicate.
Comment #6
David_Rothstein commented#4: shortcut-empty-set-937380-4.patch queued for re-testing.
Comment #7
Bojhan commentedIs this RTBC?
Comment #8
swentel commentedReroll
Comment #9
Bojhan commentedGreat to see this standard being used :)
Comment #10
webchickLooks like this fixes a usability issue, as well as an error. Good stuff.
Committed and pushed to 7.x and 8.x.
Comment #11
sivaji_ganesh_jojodae commentedLooks 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.
Comment #12
swentel commented@sivaji , see #1271026: Name of shortcut should be required for the empty string input, weird this never has been reported yet.
Comment #13
Bojhan commentedActually, the only reason that was introduced is the limit. When that doesn't apply, it should be ok to remove that whole concept.