From a2612622e8cabbe59794735dfb1ac55addfd5d36 Mon Sep 17 00:00:00 2001 From: WoH Date: Fri, 30 Nov 2018 01:26:22 +0100 Subject: [PATCH] Prevent unintended mouse keys from firing click events Firefox fires click events on left-, right- and scroll-wheel (any non-primary mouse key) clicks while other browsers don't. --- actionview/CHANGELOG.md | 13 +++++++ .../rails-ujs/features/remote.coffee | 7 +++- .../assets/javascripts/rails-ujs/start.coffee | 8 ++-- .../test/ujs/public/test/data-disable-with.js | 19 ++++++++++ .../test/ujs/public/test/data-disable.js | 19 ++++++++++ .../test/ujs/public/test/data-remote.js | 38 +++++++++++++++++++ actionview/test/ujs/public/test/settings.js | 2 +- 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 } }