Repository: zulip/zulip
Pull request: compose: Allow URLs to be pasted onto selected text.
This PR enables URL-pasting onto selected compose text. The logic checks for the presence of a URL on the clipboard following a `"paste"` event, and equally for a text selection in the compose area. Absent either of those things, no action is taken. Otherwise, a Markdown string of the selected text is formatted using the selection in the textarea, and the cursor is placed at the end of the new Markdown string.

The PR opens with a testing-related prep commit to remove brittle, self-referential event-testing logic from the copy-and-paste tests, which will break anyway once there is need to reach into the event object--as this PR does in retrieving the textarea in question.

Fixes: #18692

[Recent CZO discussion](https://chat.zulip.org/#narrow/stream/137-feedback/topic/Linkify.20text.20when.20pasting.20an.20url/near/1608530)

**Compose area:**
![copy-and-paste-compose](https://github.com/zulip/zulip/assets/170719/758d5f28-680b-4d7a-8351-2c1b2192668d)

**Edit area:**
![copy-and-paste-edit](https://github.com/zulip/zulip/assets/170719/9f9f8efb-0b14-445a-a347-f58da2c5989c)


<details>
<summary>Self-review checklist</summary>

<!-- Prior to submitting a PR, follow our step-by-step guide to review your own code:
https://zulip.readthedocs.io/en/latest/contributing/code-reviewing.html#how-to-review-code -->

<!-- Once you create the PR, check off all the steps below that you have completed.
If any of these steps are not relevant or you have not completed, leave them unchecked.-->

- [x] [Self-reviewed](https://zulip.readthedocs.io/en/latest/contributing/code-reviewing.html#how-to-review-code) the changes for clarity and maintainability
      (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

- [x] Explains differences from previous plans (e.g., issue description).
- [x] Highlights technical choices and bugs encountered.
- [x] Calls out remaining decisions and concerns.
- [x] Automated tests verify logic where appropriate.

Individual commits are ready for review (see [commit discipline](https://zulip.readthedocs.io/en/latest/contributing/commit-discipline.html)).

- [x] Each commit is a coherent idea.
- [x] Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

- [x] Visual appearance of the changes.
- [x] Responsiveness and internationalization.
- [x] Strings and tooltips.
- [x] End-to-end functionality of buttons, interactions and flows.
- [x] Corner cases, error conditions, and easily imagined bugs.
</details>



Problem statement:
Paste URL to create a named link
At present, highlighting text in the compose box and pasting a URL replaces the text with that URL.

Instead, when a user highlights text in the compose and pastes a URL, it should turn the highlighted text into a named link to that URL.

It's a really nice UX for making links, which you can try out in a GitHub comment or in Dropbox Paper to get a feel for it.

Hints from issue discussion:
Hello @zulip/server-compose members, this issue was labeled with the "area: compose" label, so you may want to check it out!

<!-- areaLabelAddition -->
I really like this idea actually. This is a very helpful feature of dropbox. 
@zulipbot claim
Reopened by #19180.
**ERROR:** This active issue has no assignee.
**ERROR:** This active issue has no assignee.
**ERROR:** This active issue has no assignee.
**ERROR:** This active issue has no assignee.
@zulipbot remove "in progress"
@karlstolley You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!

Commit message: copy_paste: Strip back tests to only test handler.

This gets us out of the brittle business of trying to mock a
complex event like "paste"--which mocking basically means we
are testing against the mock more than a real event.

The test name is also changed to clarify the handler being
tested.

See CZO discussion behind this change:
https://chat.zulip.org/#narrow/stream/43-automated-testing/topic/mocking.20browser.20events.3F/near/1615110

Patch to review:
diff --git a/web/tests/copy_and_paste.test.js b/web/tests/copy_and_paste.test.js
index 85c16e37dd0a3..d85c95307e0c0 100644
--- a/web/tests/copy_and_paste.test.js
+++ b/web/tests/copy_and_paste.test.js
@@ -2,43 +2,13 @@
 
 const {strict: assert} = require("assert");
 
-const {JSDOM} = require("jsdom");
-
-const {mock_esm, set_global, zrequire} = require("./lib/namespace");
-const jquery = require("./lib/real_jquery");
+const {zrequire} = require("./lib/namespace");
 const {run_test} = require("./lib/test");
 const {page_params} = require("./lib/zpage_params");
 
-const {window} = new JSDOM("<!DOCTYPE html><p>Hello world</p>");
-
-const {document} = window;
-const $ = jquery(window);
-
-const compose_ui = mock_esm("../src/compose_ui");
-set_global("document", document);
-
 const copy_and_paste = zrequire("copy_and_paste");
 
-// Super stripped down version of the code in the drag-mock library
-// https://github.com/andywer/drag-mock/blob/6d46c7c0ffd6a4d685e6612a90cd58cda80f30fc/src/DataTransfer.js
-class DataTransfer {
-    dataByFormat = {};
-    getData(dataFormat) {
-        return this.dataByFormat[dataFormat];
-    }
-    setData(dataFormat, data) {
-        this.dataByFormat[dataFormat] = data;
-    }
-}
-
-const createPasteEvent = function () {
-    const clipboardData = new DataTransfer();
-    const pasteEvent = new window.Event("paste");
-    pasteEvent.clipboardData = clipboardData;
-    return new $.Event(pasteEvent);
-};
-
-run_test("paste_handler", () => {
+run_test("paste_handler_converter", () => {
     page_params.development_environment = true;
     let input =
         '<meta http-equiv="content-type" content="text/html; charset=utf-8"><span style="color: hsl(0, 0%, 13%); font-family: arial, sans-serif; font-size: 12.8px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: hsl(0, 0%, 100%); text-decoration-style: initial; text-decoration-color: initial;"><span> </span>love the<span> </span><b>Zulip</b><b> </b></span><b style="color: hsl(0, 0%, 13%); font-family: arial, sans-serif; font-size: 12.8px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: hsl(0, 0%, 100%); text-decoration-style: initial; text-decoration-color: initial;">Organization</b><span style="color: hsl(0, 0%, 13%); font-family: arial, sans-serif; font-size: 12.8px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: hsl(0, 0%, 100%); text-decoration-style: initial; text-decoration-color: initial;">.</span>';
@@ -90,22 +60,4 @@ run_test("paste_handler", () => {
         copy_and_paste.paste_handler_converter(input),
         "Test list:\n*   Item 1\n*   Item 2",
     );
-
-    let data = "<p>text</p>";
-    let event = createPasteEvent();
-    event.originalEvent.clipboardData.setData("text/html", data);
-    let insert_syntax_and_focus_called = false;
-    compose_ui.insert_syntax_and_focus = function () {
-        insert_syntax_and_focus_called = true;
-    };
-    copy_and_paste.paste_handler(event);
-    assert.ok(insert_syntax_and_focus_called);
-
-    data =
-        '<meta http-equiv="content-type" content="text/html; charset=utf-8"><img src="http://localhost:9991/thumbnail?url=user_uploads%2F1%2Fe2%2FHPMCcGWOG9rS2M4ybHN8sEzh%2Fpasted_image.png&amp;size=full"/>';
-    event = createPasteEvent();
-    event.originalEvent.clipboardData.setData("text/html", data);
-    insert_syntax_and_focus_called = false;
-    copy_and_paste.paste_handler(event);
-    assert.ok(!insert_syntax_and_focus_called);
 });


Code review comments:
diff_hunk: @@ -341,7 +341,28 @@ export function paste_handler(event) {
     }
 
     if (clipboardData.getData) {
+        const $textarea = $(event.currentTarget);
+        const paste_text = clipboardData.getData("text");
         const paste_html = clipboardData.getData("text/html");
+        // Trim the paste_text to accommodate sloppy copying
+        const trimmed_paste_text = paste_text.trim();
+
+        // Match for anything that resembles a URL, something://something
+        const url_regex = /^\w+:\/\/\S+$/;
path: web/src/copy_and_paste.js
text: @user2:
Surely we have a better "is this probably a URL" check we can do, either via native libraries or already in the codebase; @user1 do you have something to suggest?

@author:
Good point. I didn't see anything in the frontend code that looked like it was checking for URLs, so I've pushed an update using the [is-url](https://github.com/segmentio/is-url) package, which looks pretty good and has a nice set of tests.

I'm still interested in Anders's opinion, too.