I have noticed that when #ajax is attached to a submit button the right click(and all other buttons of mouse, like middle) is working as well and i dont think it should. It is because the ajax.js uses the mousedown event in default. I would like to know if there is some reason for using mousedown because i think this should work only with (left)click. THX

Comments

mibfire created an issue. See original summary.

mibfire’s picture

mibfire’s picture

Issue summary: View changes
cilefen’s picture

Title: Right click on subimt button with ajax? » Right click should not work on submit buttons with Ajax behaviors
Priority: Major » Normal
mibfire’s picture

My wrong.

"Pressing the ENTER key within a textfield triggers the click event of the form's first submit button. Triggering Ajax in this situation leads to problems, like breaking autocomplete textfields, so we bind to mousedown instead of click."

mibfire’s picture

Status: Active » Closed (fixed)
nod_’s picture

Version: 7.39 » 8.1.x-dev
Status: Closed (fixed) » Needs work
Issue tags: +JavaScript

That's an interesting issue. I dislike it when right click triggers ajax calls.

We can check which button is clicked on mousedown and we probably should.

mibfire’s picture

@node_ Yes that would be a good idea.

vprocessor’s picture

Hi guys, did you mean something like this?

andypost’s picture

Status: Needs work » Needs review

#9 suppose this needs inline comment about why event.button =! 0

The last submitted patch, 2: drupal-right-click-on-submit-2616184-2.patch, failed testing.

andypost’s picture

vprocessor’s picture

Added comment with problem description

attiks’s picture

+++ b/core/misc/ajax.js
@@ -556,6 +556,11 @@
+    // Prevent submit event on right button click

missing dot at the end

vprocessor’s picture

nod_’s picture

Need strict equals, you can use eslint to check the coding standards. https://www.drupal.org/node/2274223

nod_’s picture

Status: Needs review » Needs work
droplet’s picture

vprocessor’s picture

>>nod_ : Need strict equals, you can use eslint to check the coding standards.
done

vprocessor’s picture

Status: Needs work » Needs review

The last submitted patch, 2: drupal-right-click-on-submit-2616184-2.patch, failed testing.

nod_’s picture

Thanks :)

We might have a problem though: https://github.com/jquery/jquery/blob/master/src/event.js#L434

This needs to be tested on Firefox, chrome, IE. The mobile version for those browsers, and with a right-handed mouse.

While the code is fine it might not be possible to reliably detect left click.

Use which instead of button please

vprocessor’s picture

ok, in process

vprocessor’s picture

David_Rothstein’s picture

Version: 8.1.x-dev » 8.0.x-dev
Issue tags: +needs backport to D7

Not sure why this would need to wait until 8.1.0 - seems like a straightforward bugfix, right?

  1. The patch adds code inside eventResponse, but this is already after event.preventDefault() and event.stopPropagation() have been called. Don't we want to skip running those in the case of a non-left click? It seems to me like it would be better off in the calling code instead, i.e. somewhere in here:
        // Bind the ajaxSubmit function to the element event.
        $(ajax.element).on(element_settings.event, function (event) {
          if (!drupalSettings.ajaxTrustedUrl[ajax.url] && !Drupal.url.isLocal(ajax.url)) {
            throw new Error(Drupal.t('The callback URL is not local and not trusted: !url', {'!url': ajax.url}));
          }
          return ajax.eventResponse(this, event);
        });
    
  2. +    // Prevent submit event on right button click
    

    (minor) The comment should end with a period.

    Also it is not just about the right mouse button (the issue mentions the middle button too), nor just about form submits (right?). So I would say something more like "Prevent the Ajax command from being triggered when a mouse button other than the left button is clicked."

vprocessor’s picture

vprocessor’s picture

added point at the end of string)

Status: Needs review » Needs work

The last submitted patch, 27: only-left-click-ajax-submit-2616184-27.patch, failed testing.

vprocessor’s picture

Status: Needs work » Needs review

The last submitted patch, 2: drupal-right-click-on-submit-2616184-2.patch, failed testing.

The last submitted patch, 24: only-left-click-ajax-submit-2616184-24.patch, failed testing.

David_Rothstein’s picture

The code comment still doesn't look quite accurate to me, but otherwise the patch looks good.

vprocessor’s picture

David, I have used your comment in new patch.

Re-check it please

David_Rothstein’s picture

That looks good to me now.

However in testing this out I just discovered that (on my computer at least) clicking the middle mouse button on a regular link actually opens the link (opens it in a new tab).

Given that, I'm not sure we want to prevent the middle mouse button from triggering Ajax either.

Do we know how this patch works in various environments and for various types of users with different mouse setups?

vprocessor’s picture

>>Given that, I'm not sure we want to prevent the middle mouse button from triggering Ajax either.

Hum, yep, I think too that would be better to disable default behavior for middle button

>>Do we know how this patch works in various environments and for various types of users with different mouse setups?

I use "which" instead "button" then I think it will work properly in different environments & mouse settings

timisoreana’s picture

FileSize
7.84 KB
33.85 KB
8.34 KB

Was tested adding field on article page /node/add/article

Type of click Screenshot Actual behavior Test result
Left button left button Is working on Ajax command Pass
Middle button Middle button Is not working on Ajax command Pass
Right button right button Is not working on Ajax command Pass
timisoreana’s picture

Issue tags: -Needs manual testing
andypost’s picture

Assigned: Unassigned » nod_

@nod_ looks, this needs your approval

droplet’s picture

Patch does the trick but it may not the right way to fix this bug. The problem here is we registered `mousedown` event on ajax buttons. But what we expected is `click` event behaviors.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andrewmacpherson’s picture

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

Looking at ways to revive getting @droplet's suggestion to fix it right with @nod_
We can prevent jQuery form submit and still get all the bound submit handlers and use click.

Here is a jsfiddle as a proof of concept:
https://jsfiddle.net/9d0dtcnj/4/

This should address the concern from #634616: Various problems due to AJAX binding to mousedown instead of click

@effulgentsia says "the problem is the onSubmit() handler needs to return FALSE to prevent the browser from doing what it normally does on a submit event. but, for some weird reason, it apparently needs to do it fast enough. for example, if you stick in an "alert" in the onSubmit() handler, or even a breakpoint, then you can't stop the form from submitting."

Can someone with IE give this a test and see if they can break it?

Manuel Garcia’s picture

Here's a first attempt at the new approach

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.