Add support for an empty Pc parameter in OSC 52

[j4james]

I've recently been testing terminal support for the OSC 52 clipboard sequence, and one of the issues I noticed in DomTerm was that you don't support an empty Pc parameter. For example, I would expect the test case below to copy Hello World to the clipboard, but that doesn't currently work in DomTerm:

printf "\e]52;;SGVsbG8gV29ybGQ=\e\\"

The reason why I think it's worth supporting, is because this is one of the most common ways that apps set the clipboard with OSC 52. Some examples include tmux, the FAR file manager, and the Joe editor.

The other reason I'm raising this is because a couple of terminal devs have been discussing the idea of using the primary device attributes report to indicate support for OSC 52. We've already got six people that have agreed to the idea, and I was hoping you might be willing to support the proposal in DomTerm as well. However, the minimum requirement is that the terminal can copy to the clipboard with a Pc value of c as well as with an empty Pc value, so that is something that you would need to address first.

But if you're willing to do that, and you're interested in the DA idea, there's a mini spec you can read here. We're still fleshing out some of the details, but the basic requirements are fairly straightforward.

[j4james]

I should also mention that I looked at the DomTerm code to see how difficult it would be to support an empty Pc value and I noticed that you're looping over all the Pc characters here:

DomTerm/hlib/domterm-parser.js

Line 1930 in eefdc59

for (let i = 0; i < semi; i++) {

For a paste request (i.e. when the data field is ?), that looks like you can end up sending multiple clipboard responses, which is not the way it's supposed to work. Quoting the XTerm spec:

If the second parameter is a ? , xterm replies to the host with the selection data encoded using the same protocol. It uses the first selection found

I personally don't care much the paste functionality, because most terminals don't support it, and apps don't really need it. However, I thought it was worth mentioning because fixing that loop might also make the empty Pc issue easier to address.

[PerBothner]

For DomTerm I'd like to follow xterm.js in this respect. The reason is I have a long-term goal of replacing the terminal emulator code in DomTerm with xterm.js for various reasons. (In fact you can already do so.)

I have a preliminary patch:

diff --git a/hlib/domterm-parser.js b/hlib/domterm-parser.js
index e791f95..dbca6ab 100644
--- a/hlib/domterm-parser.js
+++ b/hlib/domterm-parser.js
@@ -1927,8 +1927,12 @@ class DTParser {
             let semi = text.indexOf(';');
             if (semi >= 0) {
                 let data = text.substring(semi+1);
-                for (let i = 0; i < semi; i++) {
+                for (let i = 0; i < Math.max(semi, 1); i++) {
                     let where = text.charAt(i);
+                    if (where === ';') // no Pc parameter:
+                        where = 's';   // default according to XTerm.
+                    if (where === 's') // meaning of 's' is 'p' or 'c';
+                        where = 'p';   // default but configurable in xterm.
                     if (data == '?') { // get
                         let send_data = (text) => {
                             term.processResponseCharacters("\x1B]52;"+where
@@ -1942,6 +1946,7 @@ class DTParser {
                                 term.reportEvent("REQUEST-CLIPBOARD-TEXT", "OSC52");
                             else
                                 navigator.clipboard.readText().then(send_data);
+                            break;
                         } else if (where == 'p') { // get primary (selection)
                             if (! term.checkPermission("get-selection"))
                                 send_data("{get-selection not allowed}");
@@ -1949,6 +1954,7 @@ class DTParser {
                                 term.reportEvent("REQUEST-SELECTION-TEXT", "OSC52");
                             else
                                 send_data(Terminal._selectionAsText());
+                            break;
                         }
                     } else { // set
                         let str = atob(data);

However, the discussion needs to resolve some issues first. (Which might result in changing the default to 'c'.)