clipboard: restructure how we capture OSC 52

Previously, we'd create a clipboard handler associated with a GUI window
and take care to pass that down to the underlying Pane whenever we
spawned a new pane.

For the mux server, instead of being associated with a GUI window, the
clipboard was a special RemoteClipboard that would send a PDU through
to the client that spawned the window.

The bug here was that when that client went away, the clipboard for
that window was broken.

If the mux server was the built-in mux in a gui process this could
leave a tab without working OSC 52 clipboard support.

This commit restructures things so that the Mux is responsible for
assigning a clipboard handler that rephrases the clipboard event
as a MuxNotification.

Both the GUI frontend and the mux server dispatcher already listen
for mux notifications and translate those events into appropriate
operations on the system clipboard or Pdus to send to the client(s).

refs: #1790

codec/src/lib.rs

@@ -405,7 +405,7 @@ macro_rules! pdu {
 /// The overall version of the codec.
 /// This must be bumped when backwards incompatible changes
 /// are made to the types and protocol.
-pub const CODEC_VERSION: usize = 20;
+pub const CODEC_VERSION: usize = 21;
 
 // Defines the Pdu enum.
 // Each struct has an explicit identifying number.

docs/changelog.md

@@ -40,6 +40,7 @@ As features stabilize some brief notes about them will accumulate here.
 * `CloseCurrentPane{confirm=false}` would leave behind a phantom tab/pane when used with the multiplexer. [#1277](https://github.com/wez/wezterm/issues/1277)
 * `CloseCurrentPane{confirm=true}` artifacts when used with the multiplexer. [#783](https://github.com/wez/wezterm/issues/783)
 * Scrollbar thumb could jump around/move out of bounds. Thanks to [@davidrios](https://github.com/davidrios)! [#1525](https://github.com/wez/wezterm/issues/1525)
+* OSC 52 could stop working for tabs/panes spawned into the GUI via the CLI. [#1790](https://github.com/wez/wezterm/issues/1790)
 
 ### 20220319-142410-0fcdea07
 

mux/src/lib.rs

@@ -24,6 +24,7 @@ use std::time::Instant;
 use termwiz::escape::csi::{DecPrivateMode, DecPrivateModeCode, Device, Mode};
 use termwiz::escape::{Action, CSI};
 use thiserror::*;
+use wezterm_term::{Clipboard, ClipboardSelection};
 #[cfg(windows)]
 use winapi::um::winsock2::{SOL_SOCKET, SO_RCVBUF, SO_SNDBUF};
 
@@ -61,6 +62,11 @@ pub enum MuxNotification {
         alert: wezterm_term::Alert,
     },
     Empty,
+    AssignClipboard {
+        pane_id: PaneId,
+        selection: ClipboardSelection,
+        clipboard: Option<String>,
+    },
 }
 
 static SUB_ID: AtomicUsize = AtomicUsize::new(0);
@@ -566,6 +572,11 @@ impl Mux {
             return Ok(());
         }
 
+        let clipboard: Arc<dyn Clipboard> = Arc::new(MuxClipboard {
+            pane_id: pane.pane_id(),
+        });
+        pane.set_clipboard(&clipboard);
+
         self.panes
             .borrow_mut()
             .insert(pane.pane_id(), Rc::clone(pane));
@@ -1067,3 +1078,24 @@ pub(crate) fn pty_size_to_terminal_size(size: portable_pty::PtySize) -> wezterm_
         pixel_height: size.pixel_height as usize,
     }
 }
+
+struct MuxClipboard {
+    pane_id: PaneId,
+}
+
+impl Clipboard for MuxClipboard {
+    fn set_contents(
+        &self,
+        selection: ClipboardSelection,
+        clipboard: Option<String>,
+    ) -> anyhow::Result<()> {
+        let mux =
+            Mux::get().ok_or_else(|| anyhow::anyhow!("MuxClipboard::set_contents: no Mux?"))?;
+        mux.notify(MuxNotification::AssignClipboard {
+            pane_id: self.pane_id,
+            selection,
+            clipboard,
+        });
+        Ok(())
+    }
+}

mux/src/window.rs

@@ -1,8 +1,6 @@
 use crate::pane::CloseReason;
 use crate::{Mux, MuxNotification, Tab, TabId};
 use std::rc::Rc;
-use std::sync::Arc;
-use wezterm_term::Clipboard;
 
 static WIN_ID: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(0);
 pub type WindowId = usize;
@@ -12,7 +10,6 @@ pub struct Window {
     tabs: Vec<Rc<Tab>>,
     active: usize,
     last_active: Option<TabId>,
-    clipboard: Option<Arc<dyn Clipboard>>,
     workspace: String,
 }
 
@@ -23,7 +20,6 @@ impl Window {
             tabs: vec![],
             active: 0,
             last_active: None,
-            clipboard: None,
             workspace: workspace.unwrap_or_else(|| {
                 Mux::get()
                     .expect("Window::new to be called on mux thread")
@@ -46,10 +42,6 @@ impl Window {
         }
     }
 
-    pub fn set_clipboard(&mut self, clipboard: &Arc<dyn Clipboard>) {
-        self.clipboard.replace(Arc::clone(clipboard));
-    }
-
     pub fn window_id(&self) -> WindowId {
         self.id
     }
@@ -60,14 +52,6 @@ impl Window {
         }
     }
 
-    fn assign_clipboard_to_tab(&self, tab: &Rc<Tab>) {
-        if let Some(clip) = self.clipboard.as_ref() {
-            if let Some(pane) = tab.get_active_pane() {
-                pane.set_clipboard(clip);
-            }
-        }
-    }
-
     fn invalidate(&self) {
         let mux = Mux::get().unwrap();
         mux.notify(MuxNotification::WindowInvalidated(self.id));
@@ -75,14 +59,12 @@ impl Window {
 
     pub fn insert(&mut self, index: usize, tab: &Rc<Tab>) {
         self.check_that_tab_isnt_already_in_window(tab);
-        self.assign_clipboard_to_tab(tab);
         self.tabs.insert(index, Rc::clone(tab));
         self.invalidate();
     }
 
     pub fn push(&mut self, tab: &Rc<Tab>) {
         self.check_that_tab_isnt_already_in_window(tab);
-        self.assign_clipboard_to_tab(tab);
         self.tabs.push(Rc::clone(tab));
         self.invalidate();
     }

wezterm-gui/src/frontend.rs

@@ -10,7 +10,7 @@ use std::cell::RefCell;
 use std::collections::BTreeMap;
 use std::rc::Rc;
 use std::sync::Arc;
-use wezterm_term::Alert;
+use wezterm_term::{Alert, ClipboardSelection};
 use wezterm_toast_notification::*;
 
 pub struct GuiFrontEnd {
@@ -41,7 +41,7 @@ impl GuiFrontEnd {
         });
         let fe = Rc::downgrade(&front_end);
         mux.subscribe(move |n| {
-            if let Some(_fe) = fe.upgrade() {
+            if let Some(fe) = fe.upgrade() {
                 match n {
                     MuxNotification::WindowWorkspaceChanged(_)
                     | MuxNotification::ActiveWorkspaceChanged(_) => {}
@@ -94,6 +94,31 @@ impl GuiFrontEnd {
                             Connection::get().unwrap().terminate_message_loop();
                         }
                     }
+                    MuxNotification::AssignClipboard {
+                        pane_id,
+                        selection,
+                        clipboard,
+                    } => {
+                        log::trace!(
+                            "set clipboard in pane {} {:?} {:?}",
+                            pane_id,
+                            selection,
+                            clipboard
+                        );
+                        if let Some(window) = fe.known_windows.borrow().keys().next() {
+                            window.set_clipboard(
+                                match selection {
+                                    ClipboardSelection::Clipboard => Clipboard::Clipboard,
+                                    ClipboardSelection::PrimarySelection => {
+                                        Clipboard::PrimarySelection
+                                    }
+                                },
+                                clipboard.unwrap_or_else(String::new),
+                            );
+                        } else {
+                            log::error!("Cannot assign clipboard as there are no windows");
+                        }
+                    }
                 }
                 true
             } else {

wezterm-gui/src/termwindow/clipboard.rs

@@ -6,50 +6,20 @@ use mux::window::WindowId as MuxWindowId;
 use mux::Mux;
 use std::rc::Rc;
 use std::sync::Arc;
-use wezterm_term::ClipboardSelection;
+use window::{Clipboard, WindowOps};
-use window::{Clipboard, Window, WindowOps};
-
-/// ClipboardHelper bridges between the window crate clipboard
-/// manipulation and the term crate clipboard interface
-#[derive(Clone)]
-pub struct ClipboardHelper {
-    pub window: Window,
-}
-
-impl wezterm_term::Clipboard for ClipboardHelper {
-    fn set_contents(
-        &self,
-        selection: ClipboardSelection,
-        data: Option<String>,
-    ) -> anyhow::Result<()> {
-        self.window.set_clipboard(
-            match selection {
-                ClipboardSelection::Clipboard => Clipboard::Clipboard,
-                ClipboardSelection::PrimarySelection => Clipboard::PrimarySelection,
-            },
-            data.unwrap_or_else(String::new),
-        );
-        Ok(())
-    }
-}
 
 impl TermWindow {
-    pub fn setup_clipboard(window: &Window, mux_window_id: MuxWindowId) -> anyhow::Result<()> {
+    pub fn setup_clipboard(mux_window_id: MuxWindowId) -> anyhow::Result<()> {
-        let clipboard: Arc<dyn wezterm_term::Clipboard> = Arc::new(ClipboardHelper {
-            window: window.clone(),
-        });
         let downloader: Arc<dyn wezterm_term::DownloadHandler> =
             Arc::new(crate::download::Downloader::new());
         let mux = Mux::get().unwrap();
 
-        let mut mux_window = mux
+        let mux_window = mux
-            .get_window_mut(mux_window_id)
+            .get_window(mux_window_id)
             .ok_or_else(|| anyhow::anyhow!("mux doesn't know about window yet!?"))?;
 
-        mux_window.set_clipboard(&clipboard);
         for tab in mux_window.iter() {
             for pos in tab.iter_panes() {
-                pos.pane.set_clipboard(&clipboard);
                 pos.pane.set_download_handler(&downloader);
             }
         }
@@ -75,6 +45,11 @@ impl TermWindow {
 
     pub fn paste_from_clipboard(&mut self, pane: &Rc<dyn Pane>, clipboard: ClipboardPasteSource) {
         let pane_id = pane.pane_id();
+        log::trace!(
+            "paste_from_clipboard in pane {} {:?}",
+            pane.pane_id(),
+            clipboard
+        );
         let window = self.window.as_ref().unwrap().clone();
         let clipboard = match clipboard {
             ClipboardPasteSource::Clipboard => Clipboard::Clipboard,

wezterm-gui/src/termwindow/mod.rs

@@ -60,7 +60,6 @@ mod render;
 pub mod resize;
 mod selection;
 pub mod spawn;
-use clipboard::ClipboardHelper;
 use prevcursor::PrevCursorPos;
 use spawn::SpawnWhere;
 
@@ -798,7 +797,7 @@ impl TermWindow {
         tw.borrow_mut().window.replace(window.clone());
 
         Self::apply_icon(&window)?;
-        Self::setup_clipboard(&window, mux_window_id)?;
+        Self::setup_clipboard(mux_window_id)?;
 
         let config_subscription = config::subscribe_to_config_reload({
             let window = window.clone();
@@ -1029,6 +1028,9 @@ impl TermWindow {
                 MuxNotification::WindowRemoved(_window_id) => {
                     // Handled by frontend
                 }
+                MuxNotification::AssignClipboard { .. } => {
+                    // Handled by frontend
+                }
                 MuxNotification::PaneAdded(_)
                 | MuxNotification::PaneRemoved(_)
                 | MuxNotification::WindowWorkspaceChanged(_)
@@ -1153,12 +1155,6 @@ impl TermWindow {
                     for tab in mux_window.iter() {
                         for pos in tab.iter_panes() {
                             if pos.pane.pane_id() == pane_id {
-                                let clipboard: Arc<dyn wezterm_term::Clipboard> =
-                                    Arc::new(ClipboardHelper {
-                                        window: window.clone(),
-                                    });
-                                pos.pane.set_clipboard(&clipboard);
-
                                 let downloader: Arc<dyn wezterm_term::DownloadHandler> =
                                     Arc::new(crate::download::Downloader::new());
                                 pos.pane.set_download_handler(&downloader);
@@ -1186,6 +1182,7 @@ impl TermWindow {
                     | Alert::PaletteChanged { .. },
                 ..
             }
+            | MuxNotification::AssignClipboard { .. }
             | MuxNotification::PaneRemoved(_)
             | MuxNotification::WindowCreated(_)
             | MuxNotification::ActiveWorkspaceChanged(_)
@@ -2252,9 +2249,6 @@ impl TermWindow {
                     let size = self.terminal_size;
                     let term_config = Arc::new(TermConfig::with_config(self.config.clone()));
                     let src_window_id = self.mux_window_id;
-                    let clipboard = ClipboardHelper {
-                        window: self.window.as_ref().unwrap().clone(),
-                    };
 
                     promise::spawn::spawn(async move {
                         if let Err(err) = Self::spawn_command_internal(
@@ -2262,7 +2256,6 @@ impl TermWindow {
                             SpawnWhere::NewWindow,
                             size,
                             src_window_id,
-                            clipboard,
                             term_config,
                         )
                         .await

wezterm-gui/src/termwindow/spawn.rs

@@ -1,4 +1,4 @@
-use crate::termwindow::{ClipboardHelper, MuxWindowId};
+use crate::termwindow::MuxWindowId;
 use anyhow::{anyhow, bail, Context};
 use config::keyassignment::{SpawnCommand, SpawnTabDomain};
 use config::TermConfig;
@@ -24,16 +24,7 @@ impl super::TermWindow {
         };
         let term_config = Arc::new(TermConfig::with_config(self.config.clone()));
 
-        Self::spawn_command_impl(
+        Self::spawn_command_impl(spawn, spawn_where, size, self.mux_window_id, term_config)
-            spawn,
-            spawn_where,
-            size,
-            self.mux_window_id,
-            ClipboardHelper {
-                window: self.window.as_ref().unwrap().clone(),
-            },
-            term_config,
-        )
     }
 
     fn spawn_command_impl(
@@ -41,21 +32,14 @@ impl super::TermWindow {
         spawn_where: SpawnWhere,
         size: PtySize,
         src_window_id: MuxWindowId,
-        clipboard: ClipboardHelper,
         term_config: Arc<TermConfig>,
     ) {
         let spawn = spawn.clone();
 
         promise::spawn::spawn(async move {
-            if let Err(err) = Self::spawn_command_internal(
+            if let Err(err) =
-                spawn,
+                Self::spawn_command_internal(spawn, spawn_where, size, src_window_id, term_config)
-                spawn_where,
+                    .await
-                size,
-                src_window_id,
-                clipboard,
-                term_config,
-            )
-            .await
             {
                 log::error!("Failed to spawn: {:#}", err);
             }
@@ -68,7 +52,6 @@ impl super::TermWindow {
         spawn_where: SpawnWhere,
         size: PtySize,
         src_window_id: MuxWindowId,
-        clipboard: ClipboardHelper,
         term_config: Arc<TermConfig>,
     ) -> anyhow::Result<()> {
         let mux = Mux::get().unwrap();
@@ -104,7 +87,6 @@ impl super::TermWindow {
             None
         };
 
-        let clipboard: Arc<dyn wezterm_term::Clipboard> = Arc::new(clipboard);
         let downloader: Arc<dyn wezterm_term::DownloadHandler> =
             Arc::new(crate::download::Downloader::new());
         let workspace = mux.active_workspace().clone();
@@ -129,7 +111,6 @@ impl super::TermWindow {
                         .await
                         .context("split_pane")?;
                     pane.set_config(term_config);
-                    pane.set_clipboard(&clipboard);
                     pane.set_download_handler(&downloader);
                 } else {
                     bail!("there is no active tab while splitting pane!?");
@@ -157,7 +138,6 @@ impl super::TermWindow {
                 // the new window being created.
                 if window_id == src_window_id {
                     pane.set_config(term_config);
-                    pane.set_clipboard(&clipboard);
                     pane.set_download_handler(&downloader);
                 }
             }

wezterm-mux-server-impl/src/dispatch.rs

@@ -96,6 +96,20 @@ where
                 }
                 handler.schedule_pane_push(pane_id);
             }
+            Ok(Item::Notif(MuxNotification::AssignClipboard {
+                pane_id,
+                selection,
+                clipboard,
+            })) => {
+                Pdu::SetClipboard(codec::SetClipboard {
+                    pane_id,
+                    clipboard,
+                    selection,
+                })
+                .encode_async(&mut stream, 0)
+                .await?;
+                stream.flush().await.context("flushing PDU to client")?;
+            }
             Ok(Item::Notif(MuxNotification::WindowRemoved(_window_id))) => {}
             Ok(Item::Notif(MuxNotification::WindowCreated(_window_id))) => {}
             Ok(Item::Notif(MuxNotification::WindowInvalidated(_window_id))) => {}

wezterm-mux-server-impl/src/sessionhandler.rs

@@ -13,7 +13,7 @@ use std::sync::{Arc, Mutex};
 use std::time::Instant;
 use termwiz::surface::SequenceNo;
 use url::Url;
-use wezterm_term::terminal::{Alert, Clipboard, ClipboardSelection};
+use wezterm_term::terminal::Alert;
 use wezterm_term::StableRowIndex;
 
 #[derive(Clone)]
@@ -204,17 +204,6 @@ impl Drop for SessionHandler {
 
 impl SessionHandler {
     pub fn new(to_write_tx: PduSender) -> Self {
-        // Fixup the clipboard on the empty initial pane that is
-        // spawned into the mux
-        let mux = Mux::get().unwrap();
-        for pane in mux.iter_panes() {
-            let clip: Arc<dyn Clipboard> = Arc::new(RemoteClipboard {
-                pane_id: pane.pane_id(),
-                sender: to_write_tx.clone(),
-            });
-            pane.set_clipboard(&clip);
-        }
-
         Self {
             to_write_tx,
             per_pane: HashMap::new(),
@@ -537,19 +526,17 @@ impl SessionHandler {
             }
 
             Pdu::SpawnV2(spawn) => {
-                let sender = self.to_write_tx.clone();
                 let client_id = self.client_id.clone();
                 spawn_into_main_thread(async move {
-                    schedule_domain_spawn_v2(spawn, sender, send_response, client_id);
+                    schedule_domain_spawn_v2(spawn, send_response, client_id);
                 })
                 .detach();
             }
 
             Pdu::SplitPane(split) => {
-                let sender = self.to_write_tx.clone();
                 let client_id = self.client_id.clone();
                 spawn_into_main_thread(async move {
-                    schedule_split_pane(split, sender, send_response, client_id);
+                    schedule_split_pane(split, send_response, client_id);
                 })
                 .detach();
             }
@@ -707,60 +694,24 @@ impl SessionHandler {
 // analysis and allow things to compile.
 fn schedule_domain_spawn_v2<SND>(
     spawn: SpawnV2,
-    sender: PduSender,
     send_response: SND,
     client_id: Option<Arc<ClientId>>,
 ) where
     SND: Fn(anyhow::Result<Pdu>) + 'static,
 {
-    promise::spawn::spawn(
+    promise::spawn::spawn(async move { send_response(domain_spawn_v2(spawn, client_id).await) })
-        async move { send_response(domain_spawn_v2(spawn, sender, client_id).await) },
-    )
-    .detach();
-}
-
-fn schedule_split_pane<SND>(
-    split: SplitPane,
-    sender: PduSender,
-    send_response: SND,
-    client_id: Option<Arc<ClientId>>,
-) where
-    SND: Fn(anyhow::Result<Pdu>) + 'static,
-{
-    promise::spawn::spawn(async move { send_response(split_pane(split, sender, client_id).await) })
         .detach();
 }
 
-struct RemoteClipboard {
+fn schedule_split_pane<SND>(split: SplitPane, send_response: SND, client_id: Option<Arc<ClientId>>)
-    sender: PduSender,
+where
-    pane_id: PaneId,
+    SND: Fn(anyhow::Result<Pdu>) + 'static,
+{
+    promise::spawn::spawn(async move { send_response(split_pane(split, client_id).await) })
+        .detach();
 }
 
-impl Clipboard for RemoteClipboard {
+async fn split_pane(split: SplitPane, client_id: Option<Arc<ClientId>>) -> anyhow::Result<Pdu> {
-    fn set_contents(
-        &self,
-        selection: ClipboardSelection,
-        clipboard: Option<String>,
-    ) -> anyhow::Result<()> {
-        self.sender
-            .send(DecodedPdu {
-                serial: 0,
-                pdu: Pdu::SetClipboard(SetClipboard {
-                    pane_id: self.pane_id,
-                    clipboard,
-                    selection,
-                }),
-            })
-            .context("RemoteClipboard::set_contents send failed")?;
-        Ok(())
-    }
-}
-
-async fn split_pane(
-    split: SplitPane,
-    sender: PduSender,
-    client_id: Option<Arc<ClientId>>,
-) -> anyhow::Result<Pdu> {
     let mux = Mux::get().unwrap();
     let _identity = mux.with_identity(client_id);
 
@@ -778,12 +729,6 @@ async fn split_pane(
         )
         .await?;
 
-    let clip: Arc<dyn Clipboard> = Arc::new(RemoteClipboard {
-        pane_id: pane.pane_id(),
-        sender,
-    });
-    pane.set_clipboard(&clip);
-
     Ok::<Pdu, anyhow::Error>(Pdu::SpawnResponse(SpawnResponse {
         pane_id: pane.pane_id(),
         tab_id: tab_id,
@@ -792,11 +737,7 @@ async fn split_pane(
     }))
 }
 
-async fn domain_spawn_v2(
+async fn domain_spawn_v2(spawn: SpawnV2, client_id: Option<Arc<ClientId>>) -> anyhow::Result<Pdu> {
-    spawn: SpawnV2,
-    sender: PduSender,
-    client_id: Option<Arc<ClientId>>,
-) -> anyhow::Result<Pdu> {
     let mux = Mux::get().unwrap();
     let _identity = mux.with_identity(client_id);
 
@@ -812,12 +753,6 @@ async fn domain_spawn_v2(
         )
         .await?;
 
-    let clip: Arc<dyn Clipboard> = Arc::new(RemoteClipboard {
-        pane_id: pane.pane_id(),
-        sender,
-    });
-    pane.set_clipboard(&clip);
-
     Ok::<Pdu, anyhow::Error>(Pdu::SpawnResponse(SpawnResponse {
         pane_id: pane.pane_id(),
         tab_id: tab.tab_id(),