Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Hi,
Just noticed there's no confirmation dialog to delete editor profiles. This is somehow dangerous as it might be triggered by accident (or by CSRF).
Cheers
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | wysiwyg-DRUPAL-5.delete.patch | 4.42 KB | sun |
| #12 | wysiwyg.delete.patch | 4.46 KB | sun |
| #10 | drupal.org_wysiwyg_profile_delete_confirm-320559-10.patch.txt | 4.17 KB | markus_petrux |
| #7 | wysiwyg_profile_delete_confirm-320559-7.patch.txt | 4.22 KB | markus_petrux |
| #4 | wysiwyg_profile_delete_confirm.patch.txt | 4.22 KB | markus_petrux |
Comments
Comment #1
markus_petrux CreditAttribution: markus_petrux commentedHere's a patch against current dev snapshot.
Comment #2
markus_petrux CreditAttribution: markus_petrux commentedOpps! Minor typo in submit handler comments.
Comment #3
sunOww... too bad I recognize this patch too late. Due to #321216: Move wysiwyg_editor into wysiwyg module, this needs a re-roll now.
Comment #4
markus_petrux CreditAttribution: markus_petrux commentedOk, patch re-rolled for current dev snapshot of wysiwyg module.
In addition to adding a confirmation form (done in porevious patch), I've moved the Edit/Delete links into separate table columns (path.module also does it this way).
Comment #5
sunNice. Two minor issues:
- %name placeholder in the confirm form should not be surrounded by strong tags. This is what theme_placeholder(), resp. %token, is for.
- Instead of $arg[4] != 'delete' we want to test for !$arg[4], because this help message should only appear on the overview page.
Additionally, I'm unsure whether we really need that destination-cruft in there. So better rip it for now, since we always want to redirect to the profile overview page after deleting a profile.
Comment #6
markus_petrux CreditAttribution: markus_petrux commentedOk, I'll re-roll it now. Just one question/suggestion...
hmm... what if I check for
empty($arg[4])? (no type conversion from string to bool).The rest is done, ready to be attached here, just waiting for confirmation on this check above. :)
Comment #7
markus_petrux CreditAttribution: markus_petrux commentedWell, here's the patch. In regards to the above question... In the meantime, I opted for checking for empty($arg[4]). Please, let me know if you prefer as you suggested before. ;)
Comment #8
markus_petrux CreditAttribution: markus_petrux commented:P
Comment #9
sunempty() is even better, since I do not know (yet) whether $arg is equal to arg() (which returns empty strings for undefined args) ;)
Unfortunately, there is still some destination-cruft in confirm_form() left...
Comment #10
markus_petrux CreditAttribution: markus_petrux commentedI guess the $arg stuff is just a convention, so arg() is invoked just once and it's easier to check for empty() too.
Patch re-rolled.
Comment #11
markus_petrux CreditAttribution: markus_petrux commentedComment #12
sunHint: Please always roll patches from the root folder of a given module, i.e. the root folder of Drupal core for Drupal core patches, and the root folder of a contrib module for contrib module patches. Also, you do not have to rename .patch files to .txt on drupal.org.
Thanks, works, changed some comments, re-rolled, back-ported, committed!
Comment #13
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.