Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.On the module settings, when I click on the submit button nothing happens (except the browser loads a page from the server); it's not even clear if the module saved the settings, or it found an error.
It would be better to put a message on the screen using drupal_set_message() (like any other modules do, or should do).
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | utility_system_settings_feedback_082498.patch | 809 bytes | avpaderno |
| #12 | utility_system_settings_feedback.patch | 695 bytes | avpaderno |
| #8 | system_module_settings-notify_on_submission-reroll.patch | 709 bytes | psynaptic |
| #4 | system_module_message.patch | 429 bytes | psynaptic |
| #3 | system_module_settings-notify_on_submission.patch | 683 bytes | callison |
Comments
Comment #1
psynaptic commentedDo you mean on this page: admin/settings/util/sysmods
There are only checkboxes there so not much can go wrong. Excuse me if I'm off the mark with my assumption.
Comment #2
avpadernoNormally, all setting pages shows a message, and I think that is suggested to be done in the Drupal documentation. Even a page that has only the purpose to set some values with
variable_set()(which could not know if the new values are really saved) shows a message to the user.Comment #3
callison commentedAdded notification. Please review if this will check out. I'm not sure if the user_save() function is affected by the SQL queries above it.
Comment #4
psynaptic commentedOk. Patch attached.
Edit: Seems like the above patch is much better than mine.
Comment #5
psynaptic commentedResetting as my changes overrode callison's.
Comment #6
psynaptic commentedLooks like that code is trying to save the user data twice.
Comment #7
litwol commentedpatch from #3 doesnt apply. please re-roll.
Comment #8
psynaptic commentedHow about this one?
Comment #9
litwol commentedHunk #1 succeeded at 69 with fuzz 1 (offset 7 lines).
Committed none the less. :)
Comment #10
avpaderno@callison: It's not really requested that the code checks if the values has been saved. It should just give a feedback to the user, making him understand the code to save the values has been executed; that of course, if it's not possible to check the value returned from the function called (like it happens with
variable_set()).I would never expect the code which calls
variable_set()to check if the variable has the value set by the code. It's enough the user has a feedback, that is more important if the settings page is redirect to itself after the form has been submitted.Comment #11
litwol commentedComment #12
avpadernoIt would be enough to present the message to the user.
Comment #13
litwol commentedI have already committed this. stop reopening this issue. thanks.
Comment #14
avpadernoThe alternative would be to give a message even when the settings has not been successfully saved.
Comment #15
psynaptic commentedWell, yes. It is usually enough to just display the message but it's been committed now and it seems fine as it is. :)
Comment #16
avpadernoI am sorry; the status changed automatically. Or that, or we were adding the comment at the same time.
Comment #17
psynaptic commentedIt happens when you're replying to a comment and someone changes the status before you submit.
Comment #18
avpadernoI really didn't mean to change status; I didn't understand that the patch was already committed, or I would have stopped making suggestions.
Comment #19
psynaptic commentedWe know that :)