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.Currently, misc/ahah.js contains this code to trigger ajax form submission when the Enter key is pressed on the submit button:
// If necessary, enable keyboard submission so that AHAH behaviors
// can be triggered through keyboard input as well as e.g. a mousedown
// action.
if (element_settings.keypress) {
$(element_settings.element).keypress(function(event) {
// Detect enter key.
if (event.keyCode == 13) {
$(element_settings.element).trigger(element_settings.event);
return false;
}
});
}
I see many users expecting the same behavior when tabbing to the button and pressing the Space bar. With the current behavior, this isn't supported, and the form is submitted in the "normal" way, i.e., without AHAH's ajaxSubmit, causing a page reload.
I'm entering this as a bug on the assumption that the existing code is intended to trap all methods of form submission and re-route them to ajaxSubmit, thus preventing a page reload; on that basis, allowing a "normal" form submission via the Space bar seems like a bug.
I'm new here, so if this should have been a feature request I'm fine with changing it, of course.
Thanks,
Allen
| Comment | File | Size | Author |
|---|---|---|---|
| #61 | node607240-ahah.js__61.patch | 1000 bytes | amitgoyal |
Comments
Comment #1
TwoMice CreditAttribution: TwoMice commentedChanging this to 7.x-dev, as I see this is still the case in misc/ajax.js.
Thanks,
Allen
Comment #2
brianV CreditAttribution: brianV commentedPatch attached.
AHAH behaviours should be triggered any time the form is submitted, and therefore should be triggered with every type of input that will submit the form.
Comment #3
brianV CreditAttribution: brianV commentedAdding tags
Comment #4
brianV CreditAttribution: brianV commentedAlso, simplifying title.
Comment #5
cosmicdreams CreditAttribution: cosmicdreams commentedPatch still applies, I've tested press spacebar to submit forms on a few pages and I haven't found any ill effects yet.
Comment #6
rfayUpdating title, adding ajax tag.
Comment #7
casey CreditAttribution: casey commentedLooks good to me.
Comment #8
aspilicious CreditAttribution: aspilicious commented:) quickfix
Comment #9
rfayNeeds to be reconciled with #644158: Allow AJAX submit using space bar
Comment #10
brianV CreditAttribution: brianV commented#644158: Allow AJAX submit using space bar marked as a dupe of this issue, as this one is a month older.
Not that it matters which we stick with - the patch is near identical between the two of them, although one uses event.which() while the other uses event.keyCode(). Both work correctly across all major browsers.
Comment #11
sunWe should be using event.which(), as that is purposefully provided by jQuery. See http://api.jquery.com/event.which/ for details.
Comment #12
brianV CreditAttribution: brianV commented@sun
Ah, learn something new every day. I guess the other approach was indeed better. Well, I'll add this one to the novice queue as I don't have time to roll a new patch myself ATM.
Comment #13
aspilicious CreditAttribution: aspilicious commentedstill rtbc :)
Comment #14
aspilicious CreditAttribution: aspilicious commentedComment #15
Kiphaas7 CreditAttribution: Kiphaas7 commentedCan't test right now, but does this _always_ submit a form when hitting spacebar, or only when the submit button has focus? The first option would be buggy behavior...
Comment #16
c960657 CreditAttribution: c960657 commentedThe form only submits when the submit button has focus. Setting back to RTBC.
Something along what you say actually happened in #615282: Enter key is ignored on pages with #ajax fields, though. But this is no longer an issue.
Comment #17
klausiStill applies.
Comment #18
aspilicious CreditAttribution: aspilicious commented#13: event_which_spacebar.patch queued for re-testing.
Comment #19
aspilicious CreditAttribution: aspilicious commented*Retesting old patches*
Comment #20
rfayI tested #13, and it works fine.
I think people expect this behavior and we should get this in.
+1 from me.
Comment #21
sbrattla CreditAttribution: sbrattla commentedI've tested #13 too (on 7.0-alpha6), and I can confirm that it works fine. I think that we definately should go forward with this one.
Comment #22
sun#13: event_which_spacebar.patch queued for re-testing.
Comment #23
sunAlthough badly needed, this is D8 material according to the rules (I had to learn today). It may be backported at a later point in time (though that's unlikely).
Comment #24
rfayTo put it to 8.x, at least make a case for it other than "I am on a rampage", which is not adequate.
This is a community here, not just a piece of software.
And if you succeed in getting the software to work, but break the community, you have a very big net negative.
Comment #25
sunTrust me, I know. But Drupal core maintainers only take actually required bug fixes into account for D7. Everything else is D8 material for now. I'm with you. I had to learn the hard way.
Comment #26
Kiphaas7 CreditAttribution: Kiphaas7 commentedIt is a bug fix, since it fixes quirky behavior. Or is there some nuance between "bug fix" and "required bug fix"?
I suppose that the patch being ridiculous small, and incredible easy aren't arguments for setting this back to 7 :)?
Comment #27
rfayI agree that this is not a feature but rather a bug fix. It is definitely not an API change. This patch provides *normal*, *expected* behavior. And, of course, it's been waiting for a very long time.
Moving back to 7.x to continue the conversation.
For an issue summary, the original issue description is adequate.
Comment #28
Kiphaas7 CreditAttribution: Kiphaas7 commentedRfay, was your intent to set it back to 7.x dev? Because you didn't :).
obviously I'm with rfay, but still very interested to hear why sun thinks this still should be moved to 8.x.
Comment #29
webchickBoy, that one fell off the radar. :\
Committed to HEAD. Thanks!
Comment #30
mattyoung CreditAttribution: mattyoung commentedDid anyone try this on the iPhone? iPhone [return] code is '10' so we should also be looking for '10' in addition to '13'.
Comment #31
c960657 CreditAttribution: c960657 commentedD6 backport.
Comment #32
beefzilla CreditAttribution: beefzilla commentedIn which version of Drupal 6 is this bug fixed? This bug cannot be overridden with more javascript, (unlike some of the other nuisances), if you want to use the native ahah API. For those of us who have to deliver Drupal 6 modules that don't have any caveats nor addendums, patching the core isn't an option. -_-
Comment #33
rfayThis is in "needs review" for D6. If you'd like to get it into D6, you need to test and review it, and perhaps set it to RTBC once you've done that.
Comment #34
beefzilla CreditAttribution: beefzilla commentedUsing Windows 7, this is tested and effective in the latest versions of: FireFox, Chrome, Opera, and Safari.
IE 8, however, will both run the ahah function AND submit the form.
I'm using the version that has if(event.which == 13 || event.which == 32) btw.
Comment #35
beefzilla CreditAttribution: beefzilla commentedIn response to #30, mattyoung's question - jQuery's event.which may preclude that necessity http://api.jquery.com/event.which/
Comment #36
beefzilla CreditAttribution: beefzilla commentedI now have this working properly in all major browsers. The problem was that when ahah is attached to a button, hitting the enter key or space bar will fire an onclick event, which ahah currently doesn't support. This happens in all major browsers, but all the GOOD browsers will catch the keypress event. Internet Explorer does not, only honoring the onclick event.
The solution is to check if the ahah object being created is associated with a button. If so, bind an onclick event. Otherwise it's business as usual. See patch.
Comment #37
beefzilla CreditAttribution: beefzilla commentedComment #38
rfayThis is a D6 patch that you want reviewed for D6, right? Setting to "needs review".
Comment #40
beefzilla CreditAttribution: beefzilla commentedOk, this time I read the instructions and used eclipse to make a patch using the DRUPAL6-20 branch
Comment #41
beefzilla CreditAttribution: beefzilla commentedComment #42
beefzilla CreditAttribution: beefzilla commentedGod I suck at this. Did an EOL conversion in N++
Comment #44
beefzilla CreditAttribution: beefzilla commentedOk, I fail again. This time because I didn't apply the patch against the latest checkotu, which I assume is like a checkout, except Japanese. I looked at tags: 6.20, 6.3, and even 6.9. They all had the same ahah.js file. So I figured merging code with the most recent stable release would be a good idea, i.e. 6.20. If I really went with the latest stuff, that would be drupal 7, and that doesn't even have an ahah.js file, as it's been morphed into ajax.js.
So, I guess I should check out DRUPAL6-9 and try this again. Does that make sense, drupal admins? Somebody help the n00b get his code up in 'dere.
Comment #45
aspilicious CreditAttribution: aspilicious commentedyou should use 6 dev, thats the latest checkout
Comment #46
beefzilla CreditAttribution: beefzilla commentedI do not see a branch or version marked "6 dev" in the cvs repo.
What am I missing? Please be extremely specific. Based on one article I've found, DRUPAL-6 is the branch for development. Is this correct?
Comment #47
aspilicious CreditAttribution: aspilicious commentedAccording to http://drupal.org/node/97368 DRUPAL-6 is the development version.
I tried to apply it locally and it is working perfect...
Strange...
Comment #48
beefzilla CreditAttribution: beefzilla commentedThanks. I checked out DRUPAL-6, and put the same changes in against that and made a new patch. *crosses fingers* Though I didn't see a difference between patches, I can only guess the dates have changed?
Comment #49
beefzilla CreditAttribution: beefzilla commentedOk, this is really annoying. I got this nonsense from the automated output of my last patch:
File to patch:
Skip this patch? [y]
Skipping patch.
1 out of 1 hunk ignored.
I followed the directions for making a patch. WTF. I bet this is happening because misc/ahah.js is in the Attic - it has been removed/renamed to ajax.js in drupal 7. That's my best guess. Good thing I have my custom module rigged to automatically hack the core whenever it's installed or enabled, just to get AHAH to function properly. >:(
This is an administrative problem. The QA system is busted - my patch is simple and effective. When I flip the status to "needs work," I don't mean the patch needs work!
Comment #51
c960657 CreditAttribution: c960657 commentedThe test bot currently fails all patches due to #961172: All D6 Core patches are failing.
Comment #52
c960657 CreditAttribution: c960657 commentedBeefzilla, your patches for D6 contains a click handler that was not included in the D7 patch. Is that click handler relevant for D7 also, or is it only relevant in D6?
Comment #53
casey CreditAttribution: casey commentedThen again, spacebar isn't working on a normal non-javascript/ajax form button. Why exactly do we need to deviate from the standards?
Comment #54
c960657 CreditAttribution: c960657 commentedCasey, I'm not sure I get your point? On all browsers/OS's that I know of you can press a button using the Space bar (you should move focus to the button first using the Tab key).
Comment #55
casey CreditAttribution: casey commentedUhh... you're absolutely right. I didn't actually test it when I posted my previous comment, but I remember myself testing it some months ago. Apparently I did not and now I was just talking nonsense. Sorry for the false statements.
Comment #56
beefzilla CreditAttribution: beefzilla commentedYou, like the Dos Equis "Most interesting man in the world" probably also test your code only in production.
I have been away from this forum for a while. Wonder if my patch ever went anywhere...?
Comment #57
beefzilla CreditAttribution: beefzilla commented#48: node607240-ahah.js_.patch queued for re-testing.
Comment #58
c960657 CreditAttribution: c960657 commentedPlease see my question in #52.
Comment #59
rfayUntil your patch gets to RTBC it has no chance of getting committed, sadly. So work to get it to RTBC. And then it probably only has a minor chance of getting committed. You can talk with Gabor about whether it has a chance.
Comment #60
fearlsgroove CreditAttribution: fearlsgroove commentedPatch no longer applies, tabs in patch
Comment #61
amitgoyal CreditAttribution: amitgoyal commentedPlease review attached patch as it applies cleanly.