Force exact modifiers match for mouse bindings

Fixes #3152.

6 files changed, 55 insertions, 64 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a7387fcc..423d1a98 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## 0.4.2-dev
 
+### Changed
+
+- Pressing additional modifiers for mouse bindings will no longer trigger them
+
 ### Fixed
 
 - Incorrect default config path in `--help` on Windows and macOS
diff --git a/alacritty.yml b/alacritty.yml
index f30b5707..63d1927b 100644
--- a/alacritty.yml
+++ b/alacritty.yml
@@ -413,6 +413,9 @@
 # Mouse bindings are specified as a list of objects, much like the key
 # bindings further below.
 #
+# To trigger mouse bindings when an application running within Alacritty captures the mouse, the
+# `Shift` modifier is automatically added as a requirement.
+#
 # Each mouse binding will specify a:
 #
 # - `mouse`:
diff --git a/alacritty/src/config/bindings.rs b/alacritty/src/config/bindings.rs
index abdb1727..1a9f95af 100644
--- a/alacritty/src/config/bindings.rs
+++ b/alacritty/src/config/bindings.rs
@@ -81,20 +81,14 @@ impl Default for MouseBinding {
 
 impl<T: Eq> Binding<T> {
     #[inline]
-    pub fn is_triggered_by(
-        &self,
-        mode: TermMode,
-        mods: ModifiersState,
-        input: &T,
-        relaxed: bool,
-    ) -> bool {
+    pub fn is_triggered_by(&self, mode: TermMode, mods: ModifiersState, input: &T) -> bool {
         // Check input first since bindings are stored in one big list. This is
         // the most likely item to fail so prioritizing it here allows more
         // checks to be short circuited.
         self.trigger == *input
             && mode.contains(self.mode)
             && !mode.intersects(self.notmode)
-            && (self.mods == mods || (relaxed && self.mods.relaxed_eq(mods)))
+            && (self.mods == mods)
     }
 
     #[inline]
@@ -207,18 +201,6 @@ impl From<&'static str> for Action {
     }
 }
 
-pub trait RelaxedEq<T: ?Sized = Self> {
-    fn relaxed_eq(&self, other: T) -> bool;
-}
-
-impl RelaxedEq for ModifiersState {
-    // Make sure that modifiers in the config are always present,
-    // but ignore surplus modifiers.
-    fn relaxed_eq(&self, other: Self) -> bool {
-        !*self | other == ModifiersState::all()
-    }
-}
-
 macro_rules! bindings {
     (
         KeyBinding;
@@ -507,7 +489,9 @@ impl<'a> Deserialize<'a> for ModeWrapper {
             type Value = ModeWrapper;
 
             fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-                f.write_str("Combination of AppCursor | AppKeypad, possibly with negation (~)")
+                f.write_str(
+                    "Combination of AppCursor | AppKeypad | Alt, possibly with negation (~)",
+                )
             }
 
             fn visit_str<E>(self, value: &str) -> Result<ModeWrapper, E>
@@ -522,8 +506,8 @@ impl<'a> Deserialize<'a> for ModeWrapper {
                         "~appcursor" => res.not_mode |= TermMode::APP_CURSOR,
                         "appkeypad" => res.mode |= TermMode::APP_KEYPAD,
                         "~appkeypad" => res.not_mode |= TermMode::APP_KEYPAD,
-                        "~alt" => res.not_mode |= TermMode::ALT_SCREEN,
                         "alt" => res.mode |= TermMode::ALT_SCREEN,
+                        "~alt" => res.not_mode |= TermMode::ALT_SCREEN,
                         _ => error!(target: LOG_TARGET_CONFIG, "Unknown mode {:?}", modifier),
                     }
                 }
@@ -1040,8 +1024,8 @@ mod test {
         let mods = binding.mods;
         let mode = binding.mode;
 
-        assert!(binding.is_triggered_by(mode, mods, &13, true));
-        assert!(!binding.is_triggered_by(mode, mods, &32, true));
+        assert!(binding.is_triggered_by(mode, mods, &13));
+        assert!(!binding.is_triggered_by(mode, mods, &32));
     }
 
     #[test]
@@ -1055,14 +1039,9 @@ mod test {
         let t = binding.trigger;
         let mode = binding.mode;
 
-        assert!(binding.is_triggered_by(mode, binding.mods, &t, true));
-        assert!(binding.is_triggered_by(mode, binding.mods, &t, false));
-
-        assert!(binding.is_triggered_by(mode, superset_mods, &t, true));
-        assert!(!binding.is_triggered_by(mode, superset_mods, &t, false));
-
-        assert!(!binding.is_triggered_by(mode, subset_mods, &t, true));
-        assert!(!binding.is_triggered_by(mode, subset_mods, &t, false));
+        assert!(binding.is_triggered_by(mode, binding.mods, &t));
+        assert!(!binding.is_triggered_by(mode, superset_mods, &t));
+        assert!(!binding.is_triggered_by(mode, subset_mods, &t));
     }
 
     #[test]
@@ -1073,9 +1052,9 @@ mod test {
         let t = binding.trigger;
         let mods = binding.mods;
 
-        assert!(!binding.is_triggered_by(TermMode::INSERT, mods, &t, true));
-        assert!(binding.is_triggered_by(TermMode::ALT_SCREEN, mods, &t, true));
-        assert!(binding.is_triggered_by(TermMode::ALT_SCREEN | TermMode::INSERT, mods, &t, true));
+        assert!(!binding.is_triggered_by(TermMode::INSERT, mods, &t));
+        assert!(binding.is_triggered_by(TermMode::ALT_SCREEN, mods, &t));
+        assert!(binding.is_triggered_by(TermMode::ALT_SCREEN | TermMode::INSERT, mods, &t));
     }
 
     #[test]
@@ -1086,8 +1065,8 @@ mod test {
         let t = binding.trigger;
         let mods = binding.mods;
 
-        assert!(binding.is_triggered_by(TermMode::INSERT, mods, &t, true));
-        assert!(!binding.is_triggered_by(TermMode::ALT_SCREEN, mods, &t, true));
-        assert!(!binding.is_triggered_by(TermMode::ALT_SCREEN | TermMode::INSERT, mods, &t, true));
+        assert!(binding.is_triggered_by(TermMode::INSERT, mods, &t));
+        assert!(!binding.is_triggered_by(TermMode::ALT_SCREEN, mods, &t));
+        assert!(!binding.is_triggered_by(TermMode::ALT_SCREEN | TermMode::INSERT, mods, &t));
     }
 }
diff --git a/alacritty/src/config/mod.rs b/alacritty/src/config/mod.rs
index 91ca6936..e489f5fb 100644
--- a/alacritty/src/config/mod.rs
+++ b/alacritty/src/config/mod.rs
@@ -18,7 +18,7 @@ pub mod monitor;
 mod mouse;
 mod ui_config;
 
-pub use crate::config::bindings::{Action, Binding, Key, RelaxedEq};
+pub use crate::config::bindings::{Action, Binding, Key};
 #[cfg(test)]
 pub use crate::config::mouse::{ClickHandler, Mouse};
 use crate::config::ui_config::UIConfig;
diff --git a/alacritty/src/input.rs b/alacritty/src/input.rs
index 39f54328..c15c66d6 100644
--- a/alacritty/src/input.rs
+++ b/alacritty/src/input.rs
@@ -91,20 +91,20 @@ pub trait ActionContext<T: EventListener> {
 }
 
 trait Execute<T: EventListener> {
-    fn execute<A: ActionContext<T>>(&self, ctx: &mut A, mouse_mode: bool);
+    fn execute<A: ActionContext<T>>(&self, ctx: &mut A);
 }
 
 impl<T, U: EventListener> Execute<U> for Binding<T> {
     /// Execute the action associate with this binding
     #[inline]
-    fn execute<A: ActionContext<U>>(&self, ctx: &mut A, mouse_mode: bool) {
-        self.action.execute(ctx, mouse_mode)
+    fn execute<A: ActionContext<U>>(&self, ctx: &mut A) {
+        self.action.execute(ctx)
     }
 }
 
 impl<T: EventListener> Execute<T> for Action {
     #[inline]
-    fn execute<A: ActionContext<T>>(&self, ctx: &mut A, mouse_mode: bool) {
+    fn execute<A: ActionContext<T>>(&self, ctx: &mut A) {
         match *self {
             Action::Esc(ref s) => {
                 ctx.clear_selection();
@@ -119,11 +119,8 @@ impl<T: EventListener> Execute<T> for Action {
                 paste(ctx, &text);
             },
             Action::PasteSelection => {
-                // Only paste if mouse events are not captured by an application
-                if !mouse_mode {
-                    let text = ctx.terminal_mut().clipboard().load(ClipboardType::Selection);
-                    paste(ctx, &text);
-                }
+                let text = ctx.terminal_mut().clipboard().load(ClipboardType::Selection);
+                paste(ctx, &text);
             },
             Action::Command(ref program, ref args) => {
                 trace!("Running command {} with args {:?}", program, args);
@@ -648,10 +645,10 @@ impl<'a, T: EventListener, A: ActionContext<T>> Processor<'a, T, A> {
     /// The provided mode, mods, and key must match what is allowed by a binding
     /// for its action to be executed.
     fn process_key_bindings(&mut self, input: KeyboardInput) {
+        let mods = *self.ctx.modifiers();
         let mut suppress_chars = None;
 
         for i in 0..self.ctx.config().ui_config.key_bindings.len() {
-            let mods = *self.ctx.modifiers();
             let binding = &self.ctx.config().ui_config.key_bindings[i];
 
             let key = match (binding.trigger, input.virtual_keycode) {
@@ -660,10 +657,10 @@ impl<'a, T: EventListener, A: ActionContext<T>> Processor<'a, T, A> {
                 _ => continue,
             };
 
-            if binding.is_triggered_by(*self.ctx.terminal().mode(), mods, &key, false) {
+            if binding.is_triggered_by(*self.ctx.terminal().mode(), mods, &key) {
                 // Binding was triggered; run the action
                 let binding = binding.clone();
-                binding.execute(&mut self.ctx, false);
+                binding.execute(&mut self.ctx);
 
                 // Don't suppress when there has been a `ReceiveChar` action
                 *suppress_chars.get_or_insert(true) &= binding.action != Action::ReceiveChar;
@@ -679,17 +676,20 @@ impl<'a, T: EventListener, A: ActionContext<T>> Processor<'a, T, A> {
     /// The provided mode, mods, and key must match what is allowed by a binding
     /// for its action to be executed.
     fn process_mouse_bindings(&mut self, button: MouseButton) {
+        let mods = *self.ctx.modifiers();
+        let mode = *self.ctx.terminal().mode();
+        let mouse_mode = mode.intersects(TermMode::MOUSE_MODE);
+
         for i in 0..self.ctx.config().ui_config.mouse_bindings.len() {
-            let mods = *self.ctx.modifiers();
-            let binding = &self.ctx.config().ui_config.mouse_bindings[i];
+            let mut binding = self.ctx.config().ui_config.mouse_bindings[i].clone();
 
-            if binding.is_triggered_by(*self.ctx.terminal().mode(), mods, &button, true) {
-                // binding was triggered; run the action
-                let mouse_mode_active =
-                    !mods.shift() && self.ctx.terminal().mode().intersects(TermMode::MOUSE_MODE);
+            // Require shift for all modifiers when mouse mode is active
+            if mouse_mode {
+                binding.mods |= ModifiersState::SHIFT;
+            }
 
-                let binding = binding.clone();
-                binding.execute(&mut self.ctx, mouse_mode_active);
+            if binding.is_triggered_by(mode, mods, &button) {
+                binding.execute(&mut self.ctx);
             }
         }
     }
@@ -1003,9 +1003,9 @@ mod tests {
             #[test]
             fn $name() {
                 if $triggers {
-                    assert!($binding.is_triggered_by($mode, $mods, &KEY, false));
+                    assert!($binding.is_triggered_by($mode, $mods, &KEY));
                 } else {
-                    assert!(!$binding.is_triggered_by($mode, $mods, &KEY, false));
+                    assert!(!$binding.is_triggered_by($mode, $mods, &KEY));
                 }
             }
         }
diff --git a/alacritty/src/url.rs b/alacritty/src/url.rs
index b32a6812..4c5aed2a 100644
--- a/alacritty/src/url.rs
+++ b/alacritty/src/url.rs
@@ -11,7 +11,7 @@ use alacritty_terminal::term::cell::Flags;
 use alacritty_terminal::term::color::Rgb;
 use alacritty_terminal::term::{RenderableCell, RenderableCellContent, SizeInfo};
 
-use crate::config::{Config, RelaxedEq};
+use crate::config::Config;
 use crate::event::Mouse;
 use crate::renderer::rects::{RenderLine, RenderRect};
 
@@ -155,12 +155,17 @@ impl Urls {
         mouse_mode: bool,
         selection: bool,
     ) -> Option<Url> {
+        // Require additional shift in mouse mode
+        let mut required_mods = config.ui_config.mouse.url.mods();
+        if mouse_mode {
+            required_mods |= ModifiersState::SHIFT;
+        }
+
         // Make sure all prerequisites for highlighting are met
         if selection
-            || (mouse_mode && !mods.shift())
             || !mouse.inside_grid
             || config.ui_config.mouse.url.launcher.is_none()
-            || !config.ui_config.mouse.url.mods().relaxed_eq(mods)
+            || required_mods != mods
             || mouse.left_button_state == ElementState::Pressed
         {
             return None;