aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRafael França <rafaelmfranca@gmail.com>2018-12-05 15:57:22 -0500
committerGitHub <noreply@github.com>2018-12-05 15:57:22 -0500
commitb86f65a816546ff8eea39d25b62c995c7efc21dc (patch)
tree4149da710aa6e343859eef52521a86bb71e86d60
parentd9816546494f66e76bd5012a91b872dc2096a06c (diff)
parenta2612622e8cabbe59794735dfb1ac55addfd5d36 (diff)
downloadrails-b86f65a816546ff8eea39d25b62c995c7efc21dc.tar.gz
rails-b86f65a816546ff8eea39d25b62c995c7efc21dc.tar.bz2
rails-b86f65a816546ff8eea39d25b62c995c7efc21dc.zip
Merge pull request #34573 from WoH/firefox-right-click-bug
Prevent non-primary mouse button clicks from triggering click events
-rw-r--r--actionview/CHANGELOG.md13
-rw-r--r--actionview/app/assets/javascripts/rails-ujs/features/remote.coffee7
-rw-r--r--actionview/app/assets/javascripts/rails-ujs/start.coffee8
-rw-r--r--actionview/test/ujs/public/test/data-disable-with.js19
-rw-r--r--actionview/test/ujs/public/test/data-disable.js19
-rw-r--r--actionview/test/ujs/public/test/data-remote.js38
-rw-r--r--actionview/test/ujs/public/test/settings.js2
7 files changed, 100 insertions, 6 deletions
diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
index 237a25ba4f..f32bc3ab40 100644
--- a/actionview/CHANGELOG.md
+++ b/actionview/CHANGELOG.md
@@ -1,3 +1,16 @@
+* Prevent non-primary mouse keys from triggering Rails UJS click handlers.
+ Firefox fires click events even if the click was triggered by non-primary mouse keys such as right- or scroll-wheel-clicks.
+ For example, right-clicking a link such as the one described below (with an underlying ajax request registered on click) should not cause that request to occur.
+
+ ```
+ <%= link_to 'Remote', remote_path, class: 'remote', remote: true, data: { type: :json } %>
+ ```
+
+ Fixes #34541
+
+ *Wolfgang Hobmaier*
+
+
* Prevent `ActionView::TextHelper#word_wrap` from unexpectedly stripping white space from the _left_ side of lines.
For example, given input like this:
diff --git a/actionview/app/assets/javascripts/rails-ujs/features/remote.coffee b/actionview/app/assets/javascripts/rails-ujs/features/remote.coffee
index b3448dabac..a5b61220bb 100644
--- a/actionview/app/assets/javascripts/rails-ujs/features/remote.coffee
+++ b/actionview/app/assets/javascripts/rails-ujs/features/remote.coffee
@@ -82,9 +82,12 @@ Rails.formSubmitButtonClick = (e) ->
setData(form, 'ujs:submit-button-formaction', button.getAttribute('formaction'))
setData(form, 'ujs:submit-button-formmethod', button.getAttribute('formmethod'))
-Rails.handleMetaClick = (e) ->
+Rails.preventInsignificantClick = (e) ->
link = this
method = (link.getAttribute('data-method') or 'GET').toUpperCase()
data = link.getAttribute('data-params')
metaClick = e.metaKey or e.ctrlKey
- e.stopImmediatePropagation() if metaClick and method is 'GET' and not data
+ insignificantMetaClick = metaClick and method is 'GET' and not data
+ primaryMouseKey = e.button is 0
+ e.stopImmediatePropagation() if not primaryMouseKey or insignificantMetaClick
+
diff --git a/actionview/app/assets/javascripts/rails-ujs/start.coffee b/actionview/app/assets/javascripts/rails-ujs/start.coffee
index 32a915ac0b..5c1214df59 100644
--- a/actionview/app/assets/javascripts/rails-ujs/start.coffee
+++ b/actionview/app/assets/javascripts/rails-ujs/start.coffee
@@ -3,8 +3,8 @@
getData, $
refreshCSRFTokens, CSRFProtection
enableElement, disableElement, handleDisabledElement
- handleConfirm
- handleRemote, formSubmitButtonClick, handleMetaClick
+ handleConfirm, preventInsignificantClick
+ handleRemote, formSubmitButtonClick,
handleMethod
} = Rails
@@ -35,13 +35,14 @@ Rails.start = ->
delegate document, Rails.buttonDisableSelector, 'ajax:complete', enableElement
delegate document, Rails.buttonDisableSelector, 'ajax:stopped', enableElement
+ delegate document, Rails.linkClickSelector, 'click', preventInsignificantClick
delegate document, Rails.linkClickSelector, 'click', handleDisabledElement
delegate document, Rails.linkClickSelector, 'click', handleConfirm
- delegate document, Rails.linkClickSelector, 'click', handleMetaClick
delegate document, Rails.linkClickSelector, 'click', disableElement
delegate document, Rails.linkClickSelector, 'click', handleRemote
delegate document, Rails.linkClickSelector, 'click', handleMethod
+ delegate document, Rails.buttonClickSelector, 'click', preventInsignificantClick
delegate document, Rails.buttonClickSelector, 'click', handleDisabledElement
delegate document, Rails.buttonClickSelector, 'click', handleConfirm
delegate document, Rails.buttonClickSelector, 'click', disableElement
@@ -60,6 +61,7 @@ Rails.start = ->
delegate document, Rails.formSubmitSelector, 'ajax:send', disableElement
delegate document, Rails.formSubmitSelector, 'ajax:complete', enableElement
+ delegate document, Rails.formInputClickSelector, 'click', preventInsignificantClick
delegate document, Rails.formInputClickSelector, 'click', handleDisabledElement
delegate document, Rails.formInputClickSelector, 'click', handleConfirm
delegate document, Rails.formInputClickSelector, 'click', formSubmitButtonClick
diff --git a/actionview/test/ujs/public/test/data-disable-with.js b/actionview/test/ujs/public/test/data-disable-with.js
index 645ad494c3..b5684e0938 100644
--- a/actionview/test/ujs/public/test/data-disable-with.js
+++ b/actionview/test/ujs/public/test/data-disable-with.js
@@ -322,6 +322,25 @@ asyncTest('ctrl-clicking on a link does not disables the link', 6, function() {
start()
})
+asyncTest('right/mouse-wheel-clicking on a link does not disables the link', 10, function() {
+ var link = $('a[data-disable-with]')
+
+ App.checkEnabledState(link, 'Click me')
+
+ link.triggerNative('click', { button: 1 })
+ App.checkEnabledState(link, 'Click me')
+
+ link.triggerNative('click', { button: 1 })
+ App.checkEnabledState(link, 'Click me')
+
+ link.triggerNative('click', { button: 2 })
+ App.checkEnabledState(link, 'Click me')
+
+ link.triggerNative('click', { button: 2 })
+ App.checkEnabledState(link, 'Click me')
+ start()
+})
+
asyncTest('button[data-remote][data-disable-with] disables and re-enables', 6, function() {
var button = $('button[data-remote][data-disable-with]')
diff --git a/actionview/test/ujs/public/test/data-disable.js b/actionview/test/ujs/public/test/data-disable.js
index 88dc801b2f..9f84c4647e 100644
--- a/actionview/test/ujs/public/test/data-disable.js
+++ b/actionview/test/ujs/public/test/data-disable.js
@@ -250,6 +250,25 @@ asyncTest('ctrl-clicking on a link does not disables the link', 6, function() {
start()
})
+asyncTest('right/mouse-wheel-clicking on a link does not disable the link', 10, function() {
+ var link = $('a[data-disable]')
+
+ App.checkEnabledState(link, 'Click me')
+
+ link.triggerNative('click', { button: 1 })
+ App.checkEnabledState(link, 'Click me')
+
+ link.triggerNative('click', { button: 1 })
+ App.checkEnabledState(link, 'Click me')
+
+ link.triggerNative('click', { button: 2 })
+ App.checkEnabledState(link, 'Click me')
+
+ link.triggerNative('click', { button: 2 })
+ App.checkEnabledState(link, 'Click me')
+ start()
+})
+
asyncTest('button[data-remote][data-disable] disables and re-enables', 6, function() {
var button = $('button[data-remote][data-disable]')
diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js
index 3503c2cff3..55d39b0a52 100644
--- a/actionview/test/ujs/public/test/data-remote.js
+++ b/actionview/test/ujs/public/test/data-remote.js
@@ -63,6 +63,25 @@ asyncTest('ctrl-clicking on a link does not fire ajaxyness', 0, function() {
setTimeout(function() { start() }, 13)
})
+asyncTest('right/mouse-wheel-clicking on a link does not fire ajaxyness', 0, function() {
+ var link = $('a[data-remote]')
+
+ // Ideally, we'd setup an iframe to intercept normal link clicks
+ // and add a test to make sure the iframe:loaded event is triggered.
+ // However, jquery doesn't actually cause a native `click` event and
+ // follow links using `trigger('click')`, it only fires bindings.
+ link
+ .removeAttr('data-params')
+ .bindNative('ajax:beforeSend', function() {
+ ok(false, 'ajax should not be triggered')
+ })
+
+ link.triggerNative('click', { button: 1 })
+ link.triggerNative('click', { button: 2 })
+
+ setTimeout(function() { start() }, 13)
+})
+
asyncTest('ctrl-clicking on a link still fires ajax for non-GET links and for links with "data-params"', 2, function() {
var link = $('a[data-remote]')
@@ -148,6 +167,25 @@ asyncTest('clicking on a button with data-remote attribute', 5, function() {
.triggerNative('click')
})
+asyncTest('right/mouse-wheel-clicking on a button with data-remote attribute does not fire ajaxyness', 0, function() {
+ var button = $('button[data-remote]')
+
+ // Ideally, we'd setup an iframe to intercept normal link clicks
+ // and add a test to make sure the iframe:loaded event is triggered.
+ // However, jquery doesn't actually cause a native `click` event and
+ // follow links using `trigger('click')`, it only fires bindings.
+ button
+ .removeAttr('data-params')
+ .bindNative('ajax:beforeSend', function() {
+ ok(false, 'ajax should not be triggered')
+ })
+
+ button.triggerNative('click', { button: 1 })
+ button.triggerNative('click', { button: 2 })
+
+ setTimeout(function() { start() }, 13)
+})
+
asyncTest('changing a select option with data-remote attribute', 5, function() {
buildSelect()
diff --git a/actionview/test/ujs/public/test/settings.js b/actionview/test/ujs/public/test/settings.js
index 05677f2595..682d044403 100644
--- a/actionview/test/ujs/public/test/settings.js
+++ b/actionview/test/ujs/public/test/settings.js
@@ -71,7 +71,7 @@ try {
} catch (e) {
_MouseEvent = function(type, options) {
var evt = document.createEvent('MouseEvents')
- evt.initMouseEvent(type, options.bubbles, options.cancelable, window, options.detail, 0, 0, 80, 20, options.ctrlKey, options.altKey, options.shiftKey, options.metaKey, 0, null)
+ evt.initMouseEvent(type, options.bubbles, options.cancelable, window, options.detail, 0, 0, 80, 20, options.ctrlKey, options.altKey, options.shiftKey, options.metaKey, options.button, null)
return evt
}
}