Enter key is ignored on pages with #ajax fields [#615282]

If the upload module is enabled, the Enter key is completely ignored on node/add/page, making it impossible to insert newlines in the body field. That's evil!

The bug is in misc/ajax.js. The intention here is to capture the keypress event for a specific button, but because element_settings.element in the following line is undefined, the event is captured for the whole page:
$(element_settings.element).keypress(function (event) {

Comment	File	Size	Author
#5	615282_element_settings.patch	597 bytes	katbailey
#2	ajax-2.patch	1.04 KB	carlos8f
	ajax-1.patch	1.56 KB	c960657

Comments

Comment #1

rfay

he,him

English

Palisade, CO, USA

commented 2 November 2009 at 05:01

Issue tags:

+rfaynovember

I'll review this in my Ajax review.

Comment #2

carlos8f commented 19 November 2009 at 01:49

Status	File	Size
new	ajax-2.patch	1.04 KB

This fix works well and I think is ready to commit. Here's an up-to-date patch.

Comment #3

carlos8f commented 25 November 2009 at 06:05

can we get someone to look at this?

Comment #4

rfay

he,him

English

Palisade, CO, USA

commented 26 November 2009 at 01:47

1. it's my understanding that upload module will be removed before D7 is released, replaced by filefield. That doesn't mean we shouldn't solve this.

+++ misc/ajax.js	19 Nov 2009 01:26:54 -0000
@@ -154,7 +154,7 @@
         // ajaxSubmit that tells the system which element got clicked to
         // trigger the submit. Without it there would be no 'op' or
         // equivalent.
-        ajax.form.clk = this.element;
+        ajax.form.clk = element;
       }

2. Does the change above have anything to do with this fix?

I can confirm that the fix works, but don't have enough understanding of the particular section of code to RTBC it.

I think we can get katbailey or effulgentsia or RobLoach to take a look at it.

If the first hunk is actually a usage comment, rather than relevant to the bug, please reroll without it.

This review is powered by Dreditor.

Comment #5

katbailey commented 26 November 2009 at 02:19

Status	File	Size
new	615282_element_settings.patch	597 bytes

After chatting with rfay about this in irc, we decided the most likely explanation is that the following line

element_settings.element = this;

just got left out of ajax.js by mistake. Putting it in fixes this bug.

Comment #6

carlos8f commented 26 November 2009 at 02:23

Status:

Needs review

» Reviewed & tested by the community

Confirmed. I don't know why I didn't think of that :) RTBC in my opinion.

Comment #7

webchick

she/they

English

Vancouver 🇨🇦

commented 26 November 2009 at 03:05

Status:

Reviewed & tested by the community

» Fixed

Spoke to rfay in IRC about this, and he's pretty sure this was just an accidentally removed line from the CTools AJAX patch. Unfortunately, we have no way of testing JS, so this bug fix is good to go as-is.

Committed to HEAD. Thanks!