This small patch adds a token at the end of the links which is checked in the flag menu callback. I added two tiny wrapper functions which could be removed to save space but IMO they are a worthy abstraction. This is how nodequeue and og have implemented the same feature.
I didn't see any other place where we emit flag links (e.g. tokens or emails). If there are some, let me know and I will add the needed code.
I also renamed template_preprocess_flag() to flag_preprocess_flag() since thats the recommended way for modules to name their preprocess functions. They don't 'own' the 'template' namespace. Be sure to flush cache when testing.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | flag_tokens5.patch | 1.83 KB | quicksketch |
| #2 | mw.patch | 2 KB | moshe weitzman |
| mw.patch | 2.27 KB | moshe weitzman |
Comments
Comment #1
quicksketchThanks for whipping this up Moshe. It's got a few problems we'll need to fix yet. We'll need to make it so that this works for AJAX-toggled links also. Right now this'll throw a JS error because we return an HTML page instead of a JSON string. Granted, this shouldn't ever really happen on a legit site, it's best to be prepared and return a value that the JS can understand.
Also, according to the handbook page on preprocessors, the module that defines the template file should use the namespace "template_", for preprocess functions, similar to how they claim the namespace "theme_" for theme functions.
Comment #2
moshe weitzman commentedChanged the process function name back. I did not know that.
The flag_page() already returns a drupal_access_denied() page in another code branch. That page issues a 403 http header which is a pretty good indication to the caling code that their request was forbidden. I would not get caught up in this returning invalid JS. There is no standard way to return JS for a 403 action.
Comment #3
quicksketchLooks good to me. Our JS also looks like it accounts for handling 403 errors already, so we're all good there. Here's a Drupal 5 port (just a change in the l() parameters).
Comment #4
mooffie commentedI haven't yet tested the patch, but I think your original concern is still valid, isn't it?
Incidentally, the logic in flag_page() has always been hard to follow (for me), and parhaps it's time to revamp it a bit (e.g., rename $result to $success, and add an 'else' to the 'if').
Comment #5
quicksketchI fixed the remaining question about JS links not erroring correctly and slightly restructured flag_page(). I think it's substantially more clear than before. You can test the error behavior by editing the link href in Firebug. Now bad tokens sent by JavaScript are returned with a JSON error message.
D5: http://cvs.drupal.org/viewcvs/drupal/contributions/modules/flag/flag.mod...
D6: http://cvs.drupal.org/viewcvs/drupal/contributions/modules/flag/flag.mod...
Thanks Moshe for taking initiative with this!
Comment #6
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.