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).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

IbnDrupal’s picture

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?

David_Rothstein’s picture

Title: errors when deleting all shortcuts » errors when submitting the shortcut set configuration page after deleting all shortcuts
Version: 7.0-beta1 » 7.x-dev
Status: Active » Needs review
FileSize
1.32 KB

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.

Sivaji_Ganesh_Jojodae’s picture

Issue tags: -error
FileSize
62.67 KB

+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 ?

David_Rothstein’s picture

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.

David_Rothstein’s picture

David_Rothstein’s picture

#4: shortcut-empty-set-937380-4.patch queued for re-testing.

Bojhan’s picture

Is this RTBC?

swentel’s picture

FileSize
3.44 KB

Reroll

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

Great to see this standard being used :)

webchick’s picture

Version: 7.x-dev » 8.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +Needs backport to D7

Looks like this fixes a usability issue, as well as an error. Good stuff.

Committed and pushed to 7.x and 8.x.

Sivaji_Ganesh_Jojodae’s picture

FileSize
60.28 KB

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.

swentel’s picture

@sivaji , see #1271026: Name of shortcut should be required for the empty string input, weird this never has been reported yet.

Bojhan’s picture

Actually, the only reason that was introduced is the limit. When that doesn't apply, it should be ok to remove that whole concept.

Status: Fixed » Closed (fixed)
Issue tags: -Needs backport to D7

Automatically closed -- issue fixed for 2 weeks with no activity.