Problem/Motivation

https://geo.coop/ reported that authenticated users, including site managers, could not see comments, although anonymous visitors could.

This problem was traced to Comment Notify JavaScript:

var radiosHolder = radios.parent().parent();

And the later radiosHolder.hide();

Steps to reproduce

Very likely theme-specific (though that is all available from https://gitlab.com/agaric/sites/geo ).

Proposed resolution

Target a class or ID (more) directly rather than using parent().parent()

Remaining tasks

  • ✅ Create merge request fixing bug
  • ✅ Test
  • ❌ Code review

User interface changes

None, except that GEO.coop will be able to use Comment Notify without the entire comments section disappearing.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#15 cn-settings.png89.12 KBAnonymous (not verified)

Issue fork comment_notify-3215875

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mlncn created an issue. See original summary.

KurtTrowbridge made their first commit to this issue’s fork.

KurtTrowbridge opened merge request !5

kurttrowbridge’s picture

kurttrowbridge
he/him
commented
Status: Active » Needs review

I thought I was having a similar issue when trying to edit comments, and a cache clear may or may not have fixed that, but regardless, I started looking into where the JavaScript came from, so I can help with that: it was updated in #3144804: notify_type show/hide broken in combination with ajax_comments, to allow the radio buttons under "Notify me when new comments are posted" to hide when the "Notify me" checkbox is unchecked, specifically when the AJAX Comments module is enabled and the comment form is loaded via AJAX.

Before that patch, the JavaScript looked for IDs – #edit-notify and #edit-notify-type – instead of classes – .comment-notify and .comment-notify-type. When the form is loaded via AJAX, the IDs sensibly become #edit-notify--VmoOMmit2Eo and #edit-notify-type--VmoOMmit2Eo (or a similar ID).

I reverted the JavaScript to the pre-#3144804: notify_type show/hide broken in combination with ajax_comments version, then adjusted the selectors so that it looks for either #edit-notify or an ID beginning with #edit-notify-- (and the same for the type-including IDs). That seems to be doing the trick for me, and functionality still works while AJAX Comments is enabled, but additional reviews of my merge request would be much appreciated, and I can't necessarily guarantee that it will solve the problem originally presented.

s.perilhou’s picture

s.perilhou commented

Problem is now online with version 1.3 !

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Looks like we've got a proposed patch here. Can you try it out and review it?

s.perilhou’s picture

s.perilhou commented

Hi @greggles,

Patch seems fine for me as comments are displayed, but I do not use AJAX on this form so I can not guarantee all the features.

Thank you for the module, btw!

shaundychko’s picture

shaundychko
Location Vancouver, Canada
commented
Status: Needs review » Reviewed & tested by the community

The patch fixes this issue, applied against 8.x-1.3.

cedewey’s picture

cedewey
he/him
Primary language English
Location Denver, CO
commented
Assigned: Unassigned » gnuget
Issue summary: View changes

Thanks all for the work on this. Assigning to gnuget to review the code.

cedewey’s picture

cedewey
he/him
Primary language English
Location Denver, CO
commented
Priority: Normal » Major

Escalating this to Major since it "Interfere with normal site visitors' use of the site." See https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-...

gnuget’s picture

gnuget
Primary language Spanish
commented

I checked the patch and looks good to me.

But not so sure to understand these two lines :

            .find("input[type=checkbox]:checked")
            .attr("checked", "checked");

It finds all the checkboxes that are `checked` and adds the attribute `checked`?

Thanks!!!

gnuget’s picture

gnuget
Primary language Spanish
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new cn-settings.png89.12 KB

The problem still exist.
When the user clicks on the button to switch receiving any comment notifications, the comment form disappears.
Drupal Version 10.2 & Comment Notify 8.x-1.x-dev & Bootstrap 5
Any idea , please ?