TINY-3199: Fixed an issue where clicking remove link, wasn't removing…

… the link in some cases
tinymce · Feb 11, 2019 · 64ea6d5 · 64ea6d5
1 parent 2d9c96a
commit 64ea6d5
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 31 deletions.
diff --git a/changelog.txt b/changelog.txt
@@ -1,4 +1,5 @@
 Version 5.0.1 (TBD)
+    Fixed an issue where clicking 'Remove link' wouldn't remove the link in certain circumstances #TINY-3199
     Fixed an issue where retrieving the selected content as text didn't create newlines #TINY-3197
     Fixed incorrect keyboard shortcuts in the Help dialog for Windows #TINY-3292
     Fixed an issue where JSON serialization could produce invalid JSON #TINY-3281

diff --git a/src/core/test/ts/browser/fmt/RemoveTrailingWhitespaceFormatTest.ts b/src/core/test/ts/browser/fmt/RemoveTrailingWhitespaceFormatTest.ts
@@ -1,24 +1,17 @@
 import { GeneralSteps, Logger, Pipeline } from '@ephox/agar';
 import { TinyApis, TinyLoader, TinyUi } from '@ephox/mcagar';
-import LinkPlugin from 'tinymce/plugins/link/Plugin';
 import Theme from 'tinymce/themes/silver/Theme';
 import { UnitTest } from '@ephox/bedrock';
 
-UnitTest.asynctest(
-  'browser.tinymce.core.fmt.RemoveTrailingWhitespaceFormatTest',
-  function () {
-    const success = arguments[arguments.length - 2];
-    const failure = arguments[arguments.length - 1];
+UnitTest.asynctest('browser.tinymce.core.fmt.RemoveTrailingWhitespaceFormatTest', (success, failure) => {
 
     Theme();
-    LinkPlugin();
 
     TinyLoader.setup(function (editor, onSuccess, onFailure) {
       const tinyApis = TinyApis(editor);
       const tinyUi = TinyUi(editor);
 
       const boldSelector = 'button[aria-label="Bold"]';
-      const unlinkSelector = 'button[title="Remove link"]';
 
       Pipeline.async({}, [
         tinyApis.sFocus,
@@ -37,19 +30,18 @@ UnitTest.asynctest(
         Logger.t('unlink with leading whitespace', GeneralSteps.sequence([
           tinyApis.sSetRawContent('<p><a href="#">a b</a></p>'),
           tinyApis.sSetSelection([0, 0, 0], 1, [0, 0, 0], 3),
-          tinyUi.sClickOnToolbar('click unlink', unlinkSelector),
+          tinyApis.sExecCommand('unlink'),
           tinyApis.sAssertContent('<p><a href="#">a</a> b</p>')
         ])),
         Logger.t('unlink with trailing whitespace', GeneralSteps.sequence([
           tinyApis.sSetRawContent('<p><a href="#">a b</a></p>'),
           tinyApis.sSetSelection([0, 0, 0], 0, [0, 0, 0], 2),
-          tinyUi.sClickOnToolbar('click unlink', unlinkSelector),
+          tinyApis.sExecCommand('unlink'),
           tinyApis.sAssertContent('<p>a <a href="#">b</a></p>')
         ]))
       ], onSuccess, onFailure);
     }, {
-      plugins: 'link',
-      toolbar: 'bold unlink',
+      toolbar: 'bold',
       base_url: '/project/js/tinymce'
     }, success, failure);
   }

diff --git a/src/plugins/link/main/ts/core/Utils.ts b/src/plugins/link/main/ts/core/Utils.ts
@@ -5,9 +5,15 @@
  * For commercial licenses see https://www.tiny.cloud/
  */
 
-import { HTMLAnchorElement } from '@ephox/dom-globals';
+import { Element, HTMLAnchorElement } from '@ephox/dom-globals';
 import Tools from 'tinymce/core/api/util/Tools';
 import Settings from '../api/Settings';
+import { Editor } from 'tinymce/core/api/Editor';
+
+export interface AttachState {
+  href?: string;
+  attach?: () => void;
+}
 
 const getHref = (elm: HTMLAnchorElement): string => {
   // Returns the real href value not the resolved a.href value
@@ -38,21 +44,21 @@ const toggleTargetRules = function (rel, isUnsafe) {
   return newRel.length ? toString(newRel) : '';
 };
 
-const trimCaretContainers = function (text) {
+const trimCaretContainers = function (text: string) {
   return text.replace(/\uFEFF/g, '');
 };
 
-const getAnchorElement = function (editor, selectedElm?) {
+const getAnchorElement = function (editor: Editor, selectedElm?: Element): HTMLAnchorElement {
   selectedElm = selectedElm || editor.selection.getNode();
   if (isImageFigure(selectedElm)) {
-    // for an image conained in a figure we look for a link inside the selected element
-    return editor.dom.select('a[href]', selectedElm)[0];
+    // for an image contained in a figure we look for a link inside the selected element
+    return editor.dom.select('a[href]', selectedElm)[0] as HTMLAnchorElement;
   } else {
-    return editor.dom.getParent(selectedElm, 'a[href]');
+    return editor.dom.getParent(selectedElm, 'a[href]') as HTMLAnchorElement;
   }
 };
 
-const getAnchorText = function (selection, anchorElm) {
+const getAnchorText = function (selection, anchorElm: HTMLAnchorElement) {
   const text = anchorElm ? (anchorElm.innerText || anchorElm.textContent) : selection.getContent({ format: 'text' });
   return trimCaretContainers(text);
 };
@@ -74,11 +80,11 @@ const isOnlyTextSelected = function (html) {
   return true;
 };
 
-const isImageFigure = function (node) {
-  return node && node.nodeName === 'FIGURE' && /\bimage\b/i.test(node.className);
+const isImageFigure = function (elm: Element) {
+  return elm && elm.nodeName === 'FIGURE' && /\bimage\b/i.test(elm.className);
 };
 
-const link = function (editor, attachState) {
+const link = function (editor: Editor, attachState: AttachState) {
   return function (data) {
     editor.undoManager.transact(function () {
       const selectedElm = editor.selection.getNode();
@@ -106,8 +112,8 @@ const link = function (editor, attachState) {
         editor.focus();
 
         if (data.hasOwnProperty('text')) {
-          if ('innerText' in anchorElm) {
-            anchorElm.innerText = data.text;
+          if (anchorElm.hasOwnProperty('innerText')) {
+            (<any> anchorElm).innerText = data.text;
           } else {
             anchorElm.textContent = data.text;
           }
@@ -130,20 +136,24 @@ const link = function (editor, attachState) {
   };
 };
 
-const unlink = function (editor) {
+const unlink = function (editor: Editor) {
   return function () {
     editor.undoManager.transact(function () {
       const node = editor.selection.getNode();
       if (isImageFigure(node)) {
         unlinkImageFigure(editor, node);
       } else {
-        editor.execCommand('unlink');
+        const anchorElm = editor.dom.getParent(node, 'a[href]', editor.getBody());
+        if (anchorElm) {
+          editor.dom.remove(anchorElm, true);
+        }
       }
+      editor.focus();
     });
   };
 };
 
-const unlinkImageFigure = function (editor, fig) {
+const unlinkImageFigure = function (editor: Editor, fig: Element) {
   let a, img;
   img = editor.dom.select('img', fig)[0];
   if (img) {
@@ -155,7 +165,7 @@ const unlinkImageFigure = function (editor, fig) {
   }
 };
 
-const linkImageFigure = function (editor, fig, attrs) {
+const linkImageFigure = function (editor: Editor, fig: Element, attrs: Record<string, any>) {
   let a, img;
   img = editor.dom.select('img', fig)[0];
   if (img) {

diff --git a/src/plugins/link/test/ts/browser/ImageFigureLinkTest.ts b/src/plugins/link/test/ts/browser/ImageFigureLinkTest.ts
@@ -30,7 +30,7 @@ UnitTest.asynctest('browser.tinymce.plugins.link.ImageFigureLinkTest', (success,
       }));
     };
 
-    const sAssertPresense = function (selector) {
+    const sAssertPresence = function (selector) {
       return Logger.t('Assert element is present',
         Assertions.sAssertPresence('Detect presence of the element', selector, TinyDom.fromDom(editor.getBody()))
       );
@@ -47,11 +47,11 @@ UnitTest.asynctest('browser.tinymce.plugins.link.ImageFigureLinkTest', (success,
         ),
         api.sSetSelection([0], 0, [0], 0),
         sLinkTheSelection(),
-        sAssertPresense({ 'figure.image > a[href="http://google.com"] > img': 1 }),
+        sAssertPresence({ 'figure.image > a[href="http://google.com"] > img': 1 }),
 
         api.sSetSelection([0], 0, [0], 0),
         sUnlinkSelection(),
-        sAssertPresense({ 'figure.image > img': 1 }),
+        sAssertPresence({ 'figure.image > img': 1 }),
         TestLinkUi.sClearHistory
       ])
     , onSuccess, onFailure);

diff --git a/src/plugins/link/test/ts/browser/RemoveLinkTest.ts b/src/plugins/link/test/ts/browser/RemoveLinkTest.ts
@@ -0,0 +1,55 @@
+import 'tinymce/themes/silver/Theme';
+
+import { Assertions, Chain, Pipeline, Log } from '@ephox/agar';
+import { UnitTest } from '@ephox/bedrock';
+import { document } from '@ephox/dom-globals';
+import { TinyApis, TinyDom, TinyLoader, TinyUi } from '@ephox/mcagar';
+import { Element } from '@ephox/sugar';
+import LinkPlugin from 'tinymce/plugins/link/Plugin';
+
+UnitTest.asynctest('browser.tinymce.plugins.link.RemoveLinkTest', (success, failure) => {
+
+  LinkPlugin();
+
+  TinyLoader.setup(function (editor, onSuccess, onFailure) {
+    const tinyApis = TinyApis(editor);
+    const tinyUi = TinyUi(editor);
+    const doc = TinyDom.fromDom(document);
+    const body = Element.fromDom(editor.getBody());
+
+    Pipeline.async({}, [
+      Log.stepsAsStep('TBA', 'Removing a link with a collapsed selection', [
+        tinyApis.sSetContent('<p><a href="http://tiny.cloud">tiny</a></p>'),
+        tinyApis.sSetSelection([0, 0, 0], 2, [0, 0, 0], 2),
+        Chain.asStep(doc, [
+          tinyUi.cTriggerContextMenu('open context menu', 'a[href="http://tiny.cloud"]', '.tox-silver-sink [role="menuitem"]')
+        ]),
+        tinyUi.sClickOnUi('Click unlink', 'div[title="Remove link"]'),
+        Assertions.sAssertPresence('Assert entire link removed', { 'a[href="http://tiny.cloud"]': 0 }, body),
+      ]),
+      Log.stepsAsStep('TBA', 'Removing a link with some text selected', [
+        tinyApis.sSetContent('<p><a href="http://tiny.cloud">tiny</a></p>'),
+        tinyApis.sSetSelection([0, 0, 0], 0, [0, 0, 0], 2),
+        Chain.asStep(doc, [
+          tinyUi.cTriggerContextMenu('open context menu', 'a[href="http://tiny.cloud"]', '.tox-silver-sink [role="menuitem"]')
+        ]),
+        tinyUi.sClickOnUi('Click unlink', 'div[title="Remove link"]'),
+        Assertions.sAssertPresence('Assert entire link removed', { 'a[href="http://tiny.cloud"]': 0 }, body),
+      ]),
+      Log.stepsAsStep('TBA', 'Removing a link from an image', [
+        tinyApis.sSetContent('<p><a href="http://tiny.cloud"><img src="http://moxiecode.cachefly.net/tinymce/v9/images/logo.png" /></a></p>'),
+        tinyApis.sSetSelection([0, 0], 0, [0, 0], 1),
+        Chain.asStep(doc, [
+          tinyUi.cTriggerContextMenu('open context menu', 'a[href="http://tiny.cloud"]', '.tox-silver-sink [role="menuitem"]')
+        ]),
+        tinyUi.sClickOnUi('Click unlink', 'div[title="Remove link"]'),
+        Assertions.sAssertPresence('Assert entire link removed', { 'a[href="http://tiny.cloud"]': 0 }, body),
+      ])
+    ], onSuccess, onFailure);
+  }, {
+    plugins: 'link',
+    toolbar: 'unlink',
+    theme: 'silver',
+    base_url: '/project/js/tinymce'
+  }, success, failure);
+});