The id that is set for _signup_status_mailer_settings_form() in signup_status_mailer.settings.inc is not cleaned up properly.
On a clients site one status is named "Godkänd" (Swedish for approved). The id for that form will be "status-settings-Godkänd" and allowed characters for id are a-z, 0-9 and hyphens.
One why to fix it is to run signup_id_safe() on the id, see patch 1.
A better solution in my opinion is to use the cid instead of the name, see patch 2.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | signup_status_mailer_id_safe_1_1.patch | 1.38 KB | frjo |
| #2 | signup_status_mailer_id_safe_2_1.patch | 1.35 KB | frjo |
| signup_status_mailer_id_safe_2.patch | 781 bytes | frjo | |
| signup_status_mailer_id_safe_1.patch | 798 bytes | frjo |
Comments
Comment #1
dwwHard to say, but I think the sanitized version of the status name is better than the numeric ID for this. A few folks in IRC agree, too. ;)
However, your patch only fixes this in 1 place, where we set the id, not where we link to that anchor. Please reroll approach #1 so that you fix both parts of signup_status_mailer.settings.inc.
Thanks!
-Derek
Comment #2
frjo commentedHere are a new version of patch 1 where signup_id_safe() is used both for the id and for the anchor.
I still think using the cid is a better solution. What happens if you name your status in Japanese? Will not signup_id_safe() strip out all the characters and replace them with multiple hyphens?
Include a new version of patch 2 also incase. This version sets the id and anchor to "status-settings-cid-[cid]".
Comment #3
dwwYeah, good point. ;) Upon closer consideration, I decided to go with the numeric IDs. However, http://drupal.org/files/issues/signup_status_mailer_id_safe_2_1.patch didn't apply cleanly. But, I just fixed the problems, tested, and committed to HEAD.
Thanks!
-Derek