Hide summary in text.js not working in jQuery 1.9+

Having same issue, and there are no js error.

This was fixed and is actually a core issue: http://cgit.drupalcode.org/drupal/commit/core/modules/text/text.js?id=dd...

We should attempt to fix it in core first as we can replace what was used in 8.x (.on()) with .bind() for it to work in all versions. If there's any push back, I guess we can fix in jQuery Update, but I'd rather not.

The reason this is happening is because in jQuery 1.8+ the .toggle() event (not effect) was depreciated and finally removed in 1.9: http://api.jquery.com/toggle-event/

Here is the backported patch for 7.x.

We're using this on http://drupalwatchdog.com/ -- works great!

I can also confirm the patch fixes the issue.

RTBC +1 for #3. The patch works with the default version of jQuery included in 7.x and with the most recent jQuery release. The changes are basically a backport of what is now in 8.0.x.

Can't see it/review it if there is no JS tag.

The return false was replaced by event.preventDefault(), it's missing event.stopPropagation(). Return false means both.

I fail to see why a backport needs another technical review. The actual code was already "reviewed" in #1799594: Update to jQuery 1.9 and is just a backport of dd85e54. The only modification to this commit is using $.bind() instead of $.on() for < 1.7 jQuery version compatibility.

In regards to the return false;: the link is being dynamically created just for the purpose of a toggle click event. It does not need event.stopPropagation(), which is also likely why it isn't in 8.x either. One of the main reasons return false; had become deprecated/discouraged is because of the push to use live/on event binding with delegated targets.

When people started converting their bind methods to this new standard, they started realizing that event.stopPropagation() was actually only needed for special cases and was overused more often than not. Thus, "don't use return false and explicitly define what it is you want to prevent/stop" was the new de facto. We don't need it here and would actually prevent someone targeting links if they needed. One can always bind the parent of this link and stop the click event from propagating there. But I seriously doubt this would ever need to happen.

This backport has been reviewed and tested by several people already.

Committed to 7.x - thanks!

While I'm not sure the move away from "return false" was necessary here, I agree it doesn't sound like it would cause any harm either.

Since this was RTBC for a long time before that, I went ahead and committed it so it can get into the upcoming release... @nod_, if you do see a major problem with that change, definitely comment here and rolling the patch back is certainly an option.

@Mark Caver: D8 also replaces the link with a proper button.

I don't really want to deal with breaking people stuff so I wanted to have stopPropagation so that we have the same behavior as before.

One can always bind the parent of this link and stop the click event from propagating there. But I seriously doubt this would ever need to happen.

Yes, but that's not the point. Anyway it's in now.

The point is/was: to backport a very specific commit (cherry-pick) dd85e54 to address this issue of "Hide summary in text.js not working in jQuery 1.9+". It was still a link then too, not a button, and still had no event.stopPropagation(). Any further backporting is unnecessary and likely to increase the chance of breaking things. The only change to this commit was the $.bind() to $.on(), which was necessary.

Well we could certainly just do the attached... I tested quickly on both new and old jQuery and (not surprisingly) everything still works.

@nod_, I didn't realize you thought the change had a realistic chance of breaking someone's existing code. If so, then certainly I'd lean towards keeping the "return false". It was in the previous code and it didn't strictly need to be changed for this issue even if the D8 patch changed it. Changing it could be discussed in a new issue...

Since you are the JavaScript maintainer I'm going to leave it up to you what to do here - if you think we should commit this followup in the next day or two to be safe I certainly can.

Yes, please commit #30 too.

Typing this in the console will show the issue:

jQuery('body').bind('click', function (e) { console.log('click'); });

Drupal 7.32: nothing is printed in the console when clicking summary link.
With patch #3: 'click' is printed in the console
With patch #3+#30: nothing is printed.

It might be overly careful but I've seen some strange JS in the edit form, there is no telling what an extra click event will mean and I'm not keen on finding it out.

OK, committed the followup to 7.x - thanks!