When I enable this module the checkbox appears below the Save/Preview (S/P) buttons consistently on Safari 5.1.3 and IE 8.0.6001.18702. On Firefox 10.0 it renders properly. I have cleared every cache (Admin_menu module/flush all caches, Configuration/Development/Performance and even the CAPTCHA Placement cache) on the server side and caches on all browsers. I disabled captcha on the form with the problem, flushed the cache again and the Notify checkbox then appears above the S/P buttons. Turn captcha back on for the form and Notify checkbox and captcha appear below S/P buttons again. Disable comment_notify and captcha appears above S/P again.
Comment | File | Size | Author |
---|---|---|---|
#14 | 1434718_dont_mess_with_buttons.patch | 657 bytes | greggles |
#13 | captcha_is_positioned_correctly.png | 21 KB | Ari Linn |
#5 | comment_notify-appears_below_submit-1434718-5.patch | 1.13 KB | erikwebb |
#3 | comment_notify-appears_below_submit-1434718-3.patch | 1.31 KB | erikwebb |
Screen Shot 2012-02-09 at 3.21.55 PM.png | 61.21 KB | crossfish |
Comments
Comment #1
crossfish CreditAttribution: crossfish commentedFirefox DOES NOT render properly as I was logged in (captcha not required) when I checked the page, so there are no weird browser issues just output weights I guess.
Comment #2
endrus CreditAttribution: endrus commentedI can confirm the bug. Is there a fix?
Comment #3
erikwebb CreditAttribution: erikwebb commentedThis appears to be a combination of issues. The Captcha issues appear to be somewhat unrelated. I've attached a patch that moves the comment notify checkbox above the actions section.
Comment #4
crossfish CreditAttribution: crossfish commentedI've gone through the steps to try and apply the patch, but it fails trying to work with a path of "vendor/drupal/sites...". I really need to spend some time working on documentation for using git at drupal.org (http://drupal.org/node/707484) because for a newbie it is missing many workflow steps and best practices for someone who just needs to simply apply a patch and not be a developer or doing commits. I did examine your code changes in the patch, but my site fails with an error as the code tries to reference ['actions']['#weight'] in this line which does not exist:
+ '#weight' => $form['actions']['#weight'] - 1,
Because this problem is related to weights, I TEMPORARILY fixed it by changing the '#weight' => 19 to a higher number and that pushed the save/preview buttons back to the bottom. I'm sure that using a hard coded weight in this context is not an acceptable solution to controlling the output flow of this form, but I'm not at a level yet to offer the right solution. Thanks so much for pointing me in the direction of the problem and getting me a work-around for the time being.
Comment #5
erikwebb CreditAttribution: erikwebb commentedYes, my patch was just created incorrectly. Here's a better patch to try.
Comment #6
endrus CreditAttribution: endrus commentedDid the patch work and was it added to the release?
Comment #7
greggles@endrus - if the patch is applied then the issue will be changed to a status of "Fixed". See http://drupal.org/patch/apply and http://drupal.org/patch/review for how to apply and review a patch. Even "it works for me" would help :)
There's an alternate proposal in #1532806: Allow changing weight of notify checkbox that seems like it might be a more flexible idea.
Comment #8
endrus CreditAttribution: endrus commentedI tried the patch in #5. Alas, it didn't work for me :(
Comment #9
gregglesI'm not sure if there's a way to do this and #1532806: Allow changing weight of notify checkbox at the same time, but if so that would be great.
Comment #10
gregglesSo, I think I'm not going to commit this.
* I don't personally like this positioning. I think it should be right before the buttons.
* The actions['#weight'] is not defined by default, as mentioned in comment #4, so this will throw php notices on a default install.
* The solution in #1532806: Allow changing weight of notify checkbox is much more flexible. I plan to use that with a weight of "1" as the default to send it low on the page.
Comment #11
gregglesOk, so it seems that setting a lower weight isn't necessarily a fix. I'm re-opening this about the specific CAPTCHA issue and hope we can continue discussion of that. Ari Linn had some comments on it over at #1532806: Allow changing weight of notify checkbox.
I wonder if the Comment Notify CSS is to blame... Can someone with this issue test that?
Comment #12
Ari Linn CreditAttribution: Ari Linn commentedI did some testing on the css matter (just commented out all the comment_notify css) and it didn't help much. I also reported this to Captcha guys and they came up with an idea that comment_notify may change submit buttons weight incorrectly and then Captcha reads an invalid setting.
Comment #13
Ari Linn CreditAttribution: Ari Linn commentedUPDATE: I commented out the
$form['actions']['#weight'] = 19;
code string in comment_notify.module (functioncomment_notify_form_comment_form_alter
) as the Captcha guys suggested and it did the trick - Captcha started displaying itself correctly (screenshot below). I don't know much of the module code structure though and most probably it's not a valid fix but at least those Captcha guys have identified the problem correctly for us.Comment #14
gregglesYeah, makes sense.
Comment #15
Ari Linn CreditAttribution: Ari Linn commented'kay, seems it's working. :-)
Comment #16
gregglesThis was added for[#1434718] and I can't reproduce the original error right now, so maybe that has been fixed somewhere else?
I'll ping on that issue.
Comment #17
gregglesI meant #1211596: Save/Preview buttons appear below node body during comment preview.
Comment #18
gregglesOk, now fixed http://drupalcode.org/project/comment_notify.git/commit/1e08a8e
Comment #20
nedjoThx for the fix. Wasn't specific to browsers or theme.
Comment #20.0
nedjoUPDATE: I incorrectly stated that Firefox 10 worked, I was logged in and captcha was not required so the page rendered the Notify checkbox in the proper location. Come back from that bunny trail... :)