redactor icon indicating copy to clipboard operation
redactor copied to clipboard

Links in lists disappearing

Open texasbaptists opened this issue 2 years ago • 16 comments

Description

When trying to make a link from an entire bullet point linking to an entry or asset, any link in the previous bullet is removed or replaced.

For others dealing with this, a workaround is to start at the bottom of lists for making links.

Steps to reproduce

  1. Edit an entry that has a redactor field.
  2. In the redactor field make a bulleted list.
  3. Make a link in the first list item.
  4. In the second list item select the whole list item (triple click on Mac).
  5. Make the selection a link to either an entry or an asset (not to a text link)
  6. Watch the link you made in step 3 disappear.

Similar steps to (probably) related problem

  1. Edit an entry that has a redactor field.
  2. In the redactor field make a bulleted list.
  3. Make a link in the first list item with the last word.
  4. In the second list item select the first word in the list item (double click on Mac).
  5. Make the selection a link to either an entry or an asset (not to a text link)
  6. Watch the link you made just made overwrite the list in step 3 be overwritten by the link you just made, including the text of the link you made in step 5.

Additional info

  • Craft version: 3.7.34
  • PHP version: 7.4.27
  • Database driver & version: MySQL 8.0.27
  • Plugins & versions: "craftcms/redactor": "2.10.4"

texasbaptists avatar Feb 24 '22 23:02 texasbaptists

@texasbaptists yeah I'm able to reproduce this, but I'm unsure of what is causing this.

I've reached out to Imperavi to see if they can see if there's anything I'm doing blatantly wrong.

andris-sevcenko avatar Feb 25 '22 09:02 andris-sevcenko

image

I think this is the same issue, the starting marker is placed wrongly

SHoogland avatar Feb 28 '22 09:02 SHoogland

@SHoogland seems that way. All I'm doing is this, though, and there's no indication on what should be done differently.

I'll muck around some more, but it's mostly just shooting in the dark.

andris-sevcenko avatar Feb 28 '22 10:02 andris-sevcenko

I'm experiencing the same issues here. Have attempted to play around with fixes based on the @croxton fix here https://github.com/craftcms/redactor/issues/331 but have come up empty unfortunately. Happy to help test a solution if anyone has any ideas. Any word from Imperavi yet?

joelzerner avatar Mar 17 '22 00:03 joelzerner

@joelzerner sadly, no :/

andris-sevcenko avatar Mar 17 '22 07:03 andris-sevcenko

What appears to be happening is that Redactor will use the first child <li> in a list as the start of the link insertion marker, regardless of the actual list item selected when inserting a link. This happens when you manually select the whole of the text of a list item, or select the list item by triple-clicking (mac).

As a workaround I've discovered it's possible to insert a space before the text content of the list item and select everything after it, and that makes Redactor behave itself. This extra space remains in the markup after inserting a link, but in most cases I don't think that will matter much. There may be a a way to remove the space subsequently (when the link is inserted), but I've not got that far.

Create a plugin config/redactor/plugins/fixlinks.js with this script:

(function($R) {
    $R.add('plugin', 'fixlinks', {
        // fixes a Redactor bug with adding links to formatted text
        onformat: function(type, nodes) {
            var sel = window.getSelection();
            if (sel) {
                var nextSibling = sel.anchorNode.nextSibling;
                if (nextSibling) {
                    sel.removeAllRanges();
                    var newRange = document.createRange();
                    newRange.selectNodeContents(nextSibling);
                    sel.addRange(newRange);
                }
            }
        },
        ondropdown: {
            // fixes a Redactor bug with links in lists
            opened: function(e, dropdown, button) {
                if (button.name == 'link') {
                    var sel = window.getSelection();
                    if (sel) {
                        if (sel.anchorNode.nodeName == 'LI' || (sel.focusOffset == 0 && sel.anchorNode.parentNode.nodeName == 'LI')) {
                            var placeholder = document.createTextNode(' ');
                            var range = document.createRange();
                            var wrappingElm = sel.anchorNode.parentNode.nodeName == 'LI' ? sel.anchorNode.parentNode : sel.anchorNode;
                            range.selectNodeContents(wrappingElm);
                            range.insertNode(placeholder);
                            range.setStart(wrappingElm.firstChild, 1);
                            sel.removeAllRanges();
                            sel.addRange(range);
                        }
                    }
                }
            }
        }
    });
})(Redactor);

And add to the plugins array in your Redactor config, e.g.:

{
"plugins": ["fixlinks"]
}

croxton avatar Mar 17 '22 13:03 croxton

Thanks for your help here @croxton however your fix doesn't seem to solve the problem for me. I've double checked that it's firing - it works when the formatting menu is opened and text is formatted but it's not doing anything when "Link" > "Link to an asset" or "Link to an entry" is selected. Do you know of any Redactor event hooks for the "Link" button, in particular when an asset is select and "Insert Link" is clicked? That's when this strange link in a list issue is happening.

joelzerner avatar Mar 23 '22 11:03 joelzerner

@joelzerner 'ondropdown' should be triggered when the link button is clicked. See: https://imperavi.com/redactor/docs/callbacks/dropdown/#s-dropdown.opened

My plugin is called whenever a button with a dropdown in the toolbar is clicked, checks if it's the link button, and alters the selection before you select a link.

This page shows you the other callbacks available: https://imperavi.com/redactor/docs/callbacks/overview/

It works for me, so perhaps you have a cached script or an older version of Redactor, or it could be a browser issue? I'm using the latest Craft, the latest version of this plugin and the latest Chrome. And a config like this:

...
  "buttons": ["html", "fullscreen", "formatting", "bold", "italic", "deleted","underline","lists", "link", "image"],
  "plugins": ["fullscreen", "table", "fixlinks"],
...

croxton avatar Mar 23 '22 12:03 croxton

@croxton It's definitely firing the latest script and isn't cached. I've got pretty much the latest version of Craft and Redactor and Chrome.

I can see it working for the "Link" > "Insert Link" method but not for the "Link" > "Link to an asset" method. That still does funky stuff with the previous list item. So that's not happening for you at all even with "Link" > "Link to an asset"?

If it's definitely resolved that for you, then I'll give it a crack with a fresh version of Craft with all the latest updates and see if I can get it working too.

joelzerner avatar Mar 23 '22 12:03 joelzerner

@joelzerner OK it looks like my solution fails when you have a single word in a list item and double click it to select.

But it does work if you have more than two words in the list item and triple click to select, or if you manually highlight a single word. Can you verify?

croxton avatar Mar 23 '22 14:03 croxton

@croxton I can verify what you've said.

For a list item:

  • A double click on a single word fails (it removes the link on the previous line)
  • A triple click on a double word works (both previous line and current line stay in tact)

For a paragraph link (ie. a series of short paragraphs instead of a list):

  • A double click on a single word fails (it removes the link on the previous line)
  • A triple click on a double word fails (it replaces the previous line with the new link and selected words)

joelzerner avatar Mar 23 '22 21:03 joelzerner

There's two further problems to solve here then: being able to recognise when a selection has been made by a double click, and checking if the parent node is a paragraph <p> in addition to the existing check for <li> (and possibly should check for any block level element). It's doable I think, but I'll need to try some things out.

croxton avatar Mar 25 '22 11:03 croxton

Are there any news about this bug?

thomasbendl avatar Apr 26 '22 15:04 thomasbendl

Thank you for the bottom to top hack. We've just come across this problem also, would be great to see some updates on this bug.

Another work around we found (depending on your needs) is to hit space after creating each <li> then create the link without highlighting anything and changing the name of the link if necessary in the text field.

DanielleHarrison avatar May 03 '22 08:05 DanielleHarrison

Any updates ?

ezawadzki avatar Dec 01 '22 10:12 ezawadzki

Any updates on this?

jarheadcore avatar May 12 '23 07:05 jarheadcore