From 52d010bb224fb76c9e6909b0edc0b9b35d7e6193 Mon Sep 17 00:00:00 2001
From: Alex Henrie <alexhenrie24@gmail.com>
Date: Sat, 5 Oct 2019 10:25:25 -0600
Subject: [PATCH] Clean up composer link insertion and deletion

Allow deleting only the selected part of a link, and modify the existing
link instead of inserting a new one if the user didn't select any text.
Also restructure the code to avoid always-true or always-false if
statements.

Fixes #591
---
 src/client/composer/composer-web-view.vala | 13 ++++++--
 src/client/composer/composer-widget.vala   |  2 +-
 ui/composer-web-view.js                    | 37 +++++++++++-----------
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala
index 0f230f7c..ee495a5b 100644
--- a/src/client/composer/composer-web-view.vala
+++ b/src/client/composer/composer-web-view.vala
@@ -344,10 +344,17 @@ public class ComposerWebView : ClientWebView {
     }
 
     /**
-     * Removes any A element at the current text cursor location.
+     * Removes the A element at the current text cursor location.
+     *
+     * If only part of the A element is selected, only that part is
+     * unlinked, possibly creating two new A elements flanking the
+     * unlinked section.
      */
-    public void delete_link() {
-        this.call.begin(Util.JS.callable("geary.deleteLink"), null);
+    public void delete_link(string selection_id) {
+        this.call.begin(
+            Util.JS.callable("geary.deleteLink").string(selection_id),
+            null
+        );
     }
 
     /**
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index 3cbc21b6..31a0dff8 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -2330,7 +2330,7 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
                 this.editor.insert_link(popover.link_uri, selection_id);
             });
         popover.link_delete.connect(() => {
-                this.editor.delete_link();
+                this.editor.delete_link(selection_id);
             });
         popover.link_open.connect(() => { link_activated(popover.link_uri); });
         return popover;
diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js
index 524850ca..a848bb1a 100644
--- a/ui/composer-web-view.js
+++ b/ui/composer-web-view.js
@@ -163,30 +163,31 @@ ComposerPageState.prototype = {
         this.selections.delete(id);
     },
     insertLink: function(href, selectionId) {
-        if (!window.getSelection().isCollapsed) {
-            // There is currently a selection, so assume the user
-            // knows what they are doing and just linkify it.
+        SelectionUtil.restore(this.selections.get(selectionId));
+        if (window.getSelection().isCollapsed) {
+            // The saved selection was empty, which means that the user is
+            // modifying an existing link instead of inserting a new one.
+            let selection = SelectionUtil.save();
+            let selected = SelectionUtil.getCursorElement();
+            SelectionUtil.selectNode(selected);
             document.execCommand("createLink", false, href);
+            SelectionUtil.restore(selection);
         } else {
-            SelectionUtil.restore(this.selections.get(selectionId));
             document.execCommand("createLink", false, href);
         }
     },
-    deleteLink: function() {
-        if (!window.getSelection().isCollapsed) {
-            // There is currently a selection, so assume the user
-            // knows what they are doing and just unlink it.
-            document.execCommand("unlink", false, null);
-        } else {
+    deleteLink: function(selectionId) {
+        SelectionUtil.restore(this.selections.get(selectionId));
+        if (window.getSelection().isCollapsed) {
+            // The saved selection was empty, which means that the user is
+            // deleting the entire existing link.
+            let selection = SelectionUtil.save();
             let selected = SelectionUtil.getCursorElement();
-            if (selected != null && selected.tagName == "A") {
-                // The current cursor element is an A, so select it
-                // since unlink requires a range
-                let selection = SelectionUtil.save();
-                SelectionUtil.selectNode(selected);
-                document.execCommand("unlink", false, null);
-                SelectionUtil.restore(selection);
-            }
+            SelectionUtil.selectNode(selected);
+            document.execCommand("unlink", false, null);
+            SelectionUtil.restore(selection);
+        } else {
+            document.execCommand("unlink", false, null);
         }
     },
     indentLine: function() {