ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Link Decorators: NoFollow not being added, creates 2 separate links

Open peppies opened this issue 4 years ago • 7 comments

📝 Provide detailed reproduction steps (if any)

I'm following this demo example: https://ckeditor.com/docs/ckeditor5/latest/features/link.html#demo-2

My goal is to have the "addTargetToExternalLinks: true" for all external links and manually have the option to add "rel='nofollow'" to some links. I customized the configuration for manually adding a NoFollow attribute to some external links:

    link: {
            // Automatically add target="_blank" and rel="noopener noreferrer" to all external links.
            addTargetToExternalLinks: true,

            decorators: [
                {
                    mode: 'manual',
                    label: 'NoFollow',
                    attributes: {
                        rel: 'nofollow'
                    }
                }
            ]
        }

✔️ Expected result

<a target="_blank" rel="noopener noreferrer nofollow" href="https://www.google.com" data-aalisten="1">test</a>

❌ Actual result

<a target="_blank" rel="noopener noreferrer" href="https://www.google.com" data-aalisten="1"><a rel="nofollow" data-aalisten="1">test</a></a>

📃 Other details

  • Browser: Chrome 80.0.3987.132
  • OS: Windows 10
  • CKEditor version: 5
  • Installed CKEditor plugins: Custom Build:

{
"name": "ckeditor5-custom-build",
"author": "CKSource",
"description": "A custom CKEditor 5 build made by the CKEditor 5 online builder.",
"version": "0.0.1",
"license": "SEE LICENSE IN LICENSE.md",
"private": true,
"devDependencies": {
"@ckeditor/ckeditor5-adapter-ckfinder": "^17.0.0",
"@ckeditor/ckeditor5-alignment": "^17.0.0",
"@ckeditor/ckeditor5-autoformat": "^17.0.0",
"@ckeditor/ckeditor5-autosave": "^17.0.0",
"@ckeditor/ckeditor5-basic-styles": "^17.0.0",
"@ckeditor/ckeditor5-block-quote": "^17.0.0",
"@ckeditor/ckeditor5-ckfinder": "^17.0.0",
"@ckeditor/ckeditor5-dev-utils": "^12.0.9",
"@ckeditor/ckeditor5-dev-webpack-plugin": "^8.0.9",
"@ckeditor/ckeditor5-editor-classic": "^17.0.0",
"@ckeditor/ckeditor5-essentials": "^17.0.0",
"@ckeditor/ckeditor5-heading": "^17.0.0",
"@ckeditor/ckeditor5-image": "^17.0.0",
"@ckeditor/ckeditor5-indent": "^17.0.0",
"@ckeditor/ckeditor5-link": "^17.0.0",
"@ckeditor/ckeditor5-list": "^17.0.0",
"@ckeditor/ckeditor5-media-embed": "^17.0.0",
"@ckeditor/ckeditor5-paragraph": "^17.0.0",
"@ckeditor/ckeditor5-paste-from-office": "^17.0.0",
"@ckeditor/ckeditor5-table": "^17.0.0",
"@ckeditor/ckeditor5-theme-lark": "^17.0.0",
"@ckeditor/ckeditor5-word-count": "^17.0.0",
"postcss-loader": "^3.0.0",
"raw-loader": "^3.1.0",
"style-loader": "^1.1.3",
"terser-webpack-plugin": "^2.3.5",
"webpack": "^4.41.6",
"webpack-cli": "^3.3.11"
},
"scripts": {
"build": "webpack --mode production"
}
}


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

peppies avatar Mar 16 '20 06:03 peppies

Hi, thanks for the report. Yes, it seems something is wrong. I checked it and I get the following results:

  • if I set rel="noopener noreferrer" getData() returns:
    <a target="_blank" rel="noopener noreferrer" href="https://www.google.com">test link</a>

  • if I set rel="noopener noreferrer nofollow" I get:
    <a target="_blank" rel="noopener noreferrer" href="https://www.google.com"><a rel="noopener noreferrer nofollow">test link</a></a>

  • if I set only rel="nofollow" I get:
    <a target="_blank" rel="noopener noreferrer" href="https://www.google.com"><a rel="nofollow">test link</a></a>

cc @Reinmar

FilipTokarski avatar Mar 17 '20 13:03 FilipTokarski

The issue comes from the conflict between addTargetToExternalLinks which sets rel="noopener noreferrer" and the other manual option which sets rel="nofollow". The treats rel as a string, not an array of options. It is not capable of merging multiple rel attributes. Hence, when two links with the same attribute (rel) are applied to one fragment of text, the result is completely broken. It could be better definitely, but you won't be able to achieve a correct result at the moment. We'd need to make quite big changes in how the engine treats attributes for this to work or workaround this in the link decorators implementation which would be ugly.

Reinmar avatar Apr 06 '20 08:04 Reinmar

The easiest workaround I can see is doing the "add target to external links" completely manually either during the conversion (downcast) or outside the editor.

Reinmar avatar Apr 06 '20 08:04 Reinmar

Colleagues, sorry, but this plugin does not make image-links EXTERNAL. I see the switcher, I switch image-link to _blank, but it is not saved. I tested at least for image, which I paste from other site.

airstarh avatar Jun 30 '20 01:06 airstarh

@airstarh, could you report a new ticket for this? This sounds like a separate bug.

Reinmar avatar Jun 30 '20 08:06 Reinmar

@Reinmar thank you for the response, Sir! I've just created the issue here, I create issue here at the 1st time, so, excuse me if I do something wrong. https://github.com/ckeditor/ckeditor5/issues/7519

airstarh avatar Jun 30 '20 11:06 airstarh

As #8230 is referring to this: We just have the very same issue and at the current point of research this is (almost) a blocker. Due to a "matured" system we must provide even several target-options to choose from like: "open in new window", "open in current window", ... and even having completely custom target attributes which can be entered by editors.

As stated in #8230 this ends up in "piling up" meaningless anchor tags (only having the target attribute set) and of course, while using toggles as given from the examples, it is bad, that you have no exclusive behavior, so that you can toggle only one of them.

Here is one output of an experiment with multiple decorators all setting the target attribute:

<a href="https://example.org" target="_blank"><a target="_top"><a target="embed">example </a></a></a>

We are in need of some solution very soon - so any hints, any workarounds would be welcome. And if possible, it would be great if to provide a workaround/solution which will work with the Links plugin rather than having an independent plugin.

mmichaelis avatar Apr 13 '21 10:04 mmichaelis

4 years and still not fixed! I have to admit that HTML is not Your priority :|

piernik avatar Feb 13 '24 11:02 piernik