ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.5k stars 3.7k forks source link

Allow setting link's target attribute (allow opening links in new tabs) #4829

Closed itelmenko closed 5 years ago

itelmenko commented 6 years ago

Original ticket description

Hello!

There are not ability to select target of link in ckeditor5 .

Please, add it

Feedback needed

Please read this discussion starting from this comment and leave your feedback (by adding a reaction) in this comment.

oleq commented 6 years ago

Hi, @itelmenko. We consider implementing this feature in the future. There is no strong ETA, though.

All we can do at this moment is add the target attribute to all links in the editor view and output data by overriding the default downcast converter.

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import {
    downcastAttributeToElement
} from '@ckeditor/ckeditor5-engine/src/conversion/downcast-converters';

const linkElementSymbol = Symbol( 'linkElement' );
const linkTarget = '_blank';

class MyLinkTargetPlugin extends Plugin {
    init() {
        const editor = this.editor;

        // Override the default downcast converter provided by the LinkEditing class.
        editor.conversion.for( 'downcast' )
            .add( downcastAttributeToElement( {
                model: 'linkHref',
                view: createLinkElement,
                priority: 'high'
            } ) );
    }
}

function createLinkElement( href, writer ) {
    const linkElement = writer.createAttributeElement( 'a', {
        href,
        target: linkTarget
    }, { priority: 5 } );

    writer.setCustomProperty( linkElementSymbol, true, linkElement );

    return linkElement;
}

Then you can simply use the plugin in editing initialization

ClassicEditor
    .create( document.querySelector( '#editor' ), {
        plugins: [ ..., MyLinkTargetPlugin ]
    } );

What sort of UI would you expect to do configure the link target?

oleq commented 6 years ago

BTW: We're working on some conversion improvements to make the code shorter and to avoid overriding the default converter (which is not safe in the long run).

cc @jodator @scofalik

itelmenko commented 6 years ago

Thanks @oleq for snippet! But I can not try it because I have not

 '@ckeditor/ckeditor5-core/src/plugin';
 '@ckeditor/ckeditor5-engine/src/conversion/downcast-converters';

How can I install it?

Also it is interesting issue: When I run npm run watch it's OK. But if I run npm run production I get:

/js/backend/message/editor.1a1e6e866c5e902a4770.js from UglifyJs
Unexpected character '`' [./~/@ckeditor/ckeditor5-build-classic/build/ckeditor.js:5,6550][/js/backend/message/editor.1a1e6e866c5e902a4770.js:93,6517]

I use Laravel 5.6 (where laravel-mix use webpack 2.7.0)

Is it webpack issue or ckeditor?

oleq commented 6 years ago

Thanks @oleq for snippet! But I can not try it because I have not

'@ckeditor/ckeditor5-core/src/plugin'; '@ckeditor/ckeditor5-engine/src/conversion/downcast-converters'; How can I install it?

Check out this guide https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/builds/guides/development/installing-plugins.html

Also it is interesting issue: When I run npm run watch it's OK. But if I run npm run production I get:

/js/backend/message/editor.1a1e6e866c5e902a4770.js from UglifyJs Unexpected character '`' [./~/@ckeditor/ckeditor5-build-classic/build/ckeditor.js:5,6550][/js/backend/message/editor.1a1e6e866c5e902a4770.js:93,6517] I use Laravel 5.6 (where laravel-mix use webpack 2.7.0)

Is it webpack issue or ckeditor?

We also noticed certain issues with UglifyJs in https://github.com/ckeditor/ckeditor5-dev/issues/371 (Webpack 4.x uses it by default). ATM I'd rather recommend you to use babel-minify-webpack-plugin which works just fine.

ssougnez commented 5 years ago

Hi,

I just found this topic while wondering how come it was not possible to add target to a link.

@oleq, you asked what kind of UI was expected for this. Why not a simple drop down with the posssible values (https://www.w3schools.com/tags/att_a_target.asp).

I guess the first implementation of this feature might not include framename as almost no one uses frame anymore anyway.

This seems to me like a mandatory feature and when you see the awesome job you did for tables, it’s really a shame that links miss such a basic functionality.

I really hope someone will find the time to implement this feature.

imkane commented 5 years ago

I'm surprised that not every single user of CKEditor would like the option to open links in a new tab. Otherwise, every link in your app will break out of the app. Isn't this like a mandatory feature?

Reinmar commented 5 years ago

I'm surprised that not every single user of CKEditor would like the option to open links in a new tab. Otherwise, every link in your app will break out of the app. Isn't this like a mandatory feature?

It depends.

CKEditor 5 can't decide automatically what should be the behaviour. Therefore, there are three options:

The first option is something that we can implement, of course, and perhaps will. We just don't have this on the roadmap yet.

The second option is what many CMS does. It's actually the best. The content output from CKEditor 5 is treated as abstract data. This data is stored in HTML format, but that's just a format (e.g. Markdown doesn't let you define target=blank as well). But from the abstraction perspective, links have no such property as how they should be opened – that's not to be defined by the data itself. That's to be defined by the system which renders that content to the end users – i.e. your backend systems.

ssougnez commented 5 years ago

I'm a bit confused by this explanation. Most of the time, ckeditor will be used to allow one or several users to enter and save content.

Therefore it is the user who decides whether a sentence is bold, underlined or... a link opens in a new window or not.

How could the system know whether the user wants the link to open in a new tab or not? If the underlying system should be able to do that, why wouldn't it be able to also decide whether a text is bold or not ? At the end, it's the user who decide, not the system (at least in my opinion but I could be wrong), otherwise, the whole formatting features would bebuseless.

imkane commented 5 years ago

A follow up question ...

This feature was available in CKEditor 4. Why was it removed from v5? Or was it just somehow decided that it wasn't important to implement early on?

Reinmar commented 5 years ago

Our opinion

How could the system know whether the user wants the link to open in a new tab or not?

The question is – how can the user know that? Usually, normal users don't understand and/or care about such things. Plus, a system can do things consistently, based on predefined rules. Users won't do that. If you want all external links to be opened in a new tab (which is what most people want), you can automate this. As I see it this is the most common scenario and therefore that'd be our first step.

However, I understand that in some specific scenarios where these really are users who make the final decision it needs to be possible to control this attribute via the balloon. That's also a feasible extension, but I'd consider this a second step.

This feature was available in CKEditor 4. Why was it removed from v5? Or was it just somehow decided that it wasn't important to implement early on?

A bit of both. When implementing CKEditor 5 we had to review the existing features and rethink most of them. CKEditor 4 (and CKEditor 3 and FCKeditor) have a long history of adding WYSIWYG features in a way which works for developers and for building websites, but makes less sense for normal users and from content semantics perspective. CKEditor 4 started turning into the WYSIWYM direction around 2013 and CKEditor 5 just follows that. It's a rich-text editor which allows WYSIWYM editing. The behaviour of links is just not part of content semantics and a technical detail which most users wouldn't normally care. Also, this happens to be in line with other modern editors – Quill, Prosemirror or Slate don't let you set the target as well. Markdown doesn't allow this too.

But I said "a bit of both" because I don't say we would never implement it. It's just something that's nice to have (because in some cases it may be necessary), but wasn't there in the roadmap to 1.0.0.

Feedback needed

What I wrote above is how we think about it. We may be wrong regarding the priorities so, please vote:

Before voting please read this discussion starting from this comment up to this one.

imkane commented 5 years ago

It's sorta funny you talk about other modern editors not having the link target feature, but you fail to talk about BY FAR the most common one out there ... the one that runs 30% of websites ... WordPress. If they ever got rid of link target ability there'd be riots lol.

For anyone like myself using CKEditor in a SaaS, setting an optional link target is absolutely crucial. I can think of numerous other applications where it would be as well. I'm still quite surprised you haven't received many, many more complaints about this.

You can probably tell that I of course voted for the "open link in a new tab" checkbox :)

FFohlin commented 5 years ago

@Reinmar To have our backend parse every text and rewrite the a elements is not an option.

daveh02 commented 5 years ago

@Reinmar When is the voting closed and the outcome presented ? When can we expect the checkbox option ?🙂

Reinmar commented 5 years ago

Note to ourselves: Let's update https://stackoverflow.com/questions/49366292/ckeditor-5-links-set-default-target-for-links-or-edit-target once we get this.

Reinmar commented 5 years ago

@Reinmar When is the voting closed and the outcome presented ? When can we expect the checkbox option ?🙂

Well :trollface: I saw many votes on the checkbox option coming from Swedes (hello from Poland, BTW ;) and newly registered users on the very first day after I asked the question. Normalizing the results with that in mind I'd say it's a draw. Most importantly though, both options got votes, which means that both are worth being implemented. When? IDK yet.

Reinmar commented 5 years ago

@Reinmar To have our backend parse every text and rewrite the a elements is not an option.

Can I ask why?

FFohlin commented 5 years ago

@Reinmar To have our backend parse every text and rewrite the a elements is not an option.

Can I ask why?

Sure. We are using blank for different reasons. We are not able to use the url to determine if the blank should be used or not. internal or external links (or other url parts) does not help us. Futhermore we try to have the wysiwyg editor to show the result as clearly as possible, and we would have to both parse the html backend, and write a little script to tell the content editors what they can expect in result. Very often they do not see the result on the site when creating the article.

In our company this became a problem for quite a few. And some of them even registered here maybe :)

Keep up the good work, we really like changes made between 4 and 5! (I think more table functionality is on the wish list for our editors soon)

imkane commented 5 years ago

This is exactly the same reason why users of our SaaS need the option to set certain links (but not all links) to open in a new tab.

Sure. We are using blank for different reasons. We are not able to use the url to determine if the blank should be used or not. internal or external links (or other url parts) does not help us. Futhermore we try to have the wysiwyg editor to show the result as clearly as possible, and we would have to both parse the html backend, and write a little script to tell the content editors what they can expect in result. Very often they do not see the result on the site when creating the article.

FYI we've had to revert back to CKEditor 4 as this lack of functionality is a deal-breaker.

ssougnez commented 5 years ago

As everyone seems to give use case for this, I'll add mine. We are using umbraco cms to allow content managers to create the content. However the site has some dynamic component made in angularjs. The problem is then that some sentence displayed by these content needs to be translated (text like "no result found". So I use the balloon editor to enable content manager to edit these label on the fly.

In this label, managers can add link but the must be able to specify how the link opens...

oleq commented 5 years ago

Related https://github.com/ckeditor/ckeditor5/issues/1514.

mlewand commented 5 years ago

We actually would like to provide means to solve both approaches:

While thinking about API, this feature can actually be generalized and not limited to assigning targets only. Other use cases include a[download], a[rel], a[ping] attributes. For instance one might want to force download for all links that are contained in, say, /uploads/ directory.

So the common feature sounds more like link decorators.

Config

config.link.decorator = [ 
    {
        // Mode could be either manual or automatic.
        mode: 'automatic',
        callback: function( url ) {
            // This should be a more precise regexp.
            return url.startswith( 'https://' );
        },
        // This is a set of attribute applied to the link in view.
        attributes: {
            rel: 'noopener noreferrer',
            target: '_blank'
        }
    },
    {
        mode: 'manual',
        label: 'Force download', // Label for the checkbox.

        attributes: {
            download: 'download'
        }
    },
    {
        mode: 'automatic',
        callback: function( url ) {
            return url.includes( '/gallery/' ) && url.endswith( '.png' );
        },
        attributes: {
            class: 'lightbox'
        }
    }
];

The config to manually handle a[target=_blank] would be the following:

config.link.decorator = [ {
    mode: 'manual',
    label: 'Open in a new window',

    attributes: {
        target: '_blank',
        manualTarget: true
    }
} ];

UI

Manual decorators should add a checkbox to the UI. We don't expect to extend this feature to provide other input controls (selects/radios).

The simplest idea would be just to put them below the url input:

Editor with the link balloon menu open

One thing to remember here is to make sure that the tab order make sense after adding these controls.

Model

In case of manual decorators the feature will need to place the state in editor model. Simply generating a new unique attribute for each decorator sounds reasonable. So that e.g. linkDecorator1 attribute results with a link that has [target=_blank].

There's no such need for the automatic decorators, as they're stateless and should be queried each time a model item is being converted to a view.

Default behavior

An open question remains whether we need to enable this by default? After thinking this case I believe that adding automatic decorator that would add a[target=_blank] to any external link might be surprising. It sounds better for me to make this an opt-in.

In that case our documentation should contain listings for both solutions.

Reinmar commented 5 years ago

It sounds better for me to make this an opt-in.

In that case our documentation should contain listings for both solutions.

👍 for opt-in and samples of as many solutions as possible.

Other notes:

msamsel commented 5 years ago

@mlewand, @Reinmar I'm wondering how specific cases should be covered with manual decorators and ranged selection.

As I can see there isn't displayed any UI for range selection on links, even if those are entirely inside single link. However it's possible to execute command on such range, and behaviour in such cases really depends from selection range.

Should this feature support somehow ranged selection, or be available exclusively for collapsed one and link creation from toolbar?

Reinmar commented 5 years ago

It should do exactly the same thing which the link command does. It must be 1:1 synced with it. In fact, I think that perhaps we should extend the link command here to ensure it always applies all the attributes.

mlewand commented 5 years ago

This feature just got merged to the master branch 🎉 You'll be able to set [target=_blank] (and any other attributes) just by changing the configuration using link decorators feature.

Basic configuration should looks something like that:

ClassicEditor
    .create( document.querySelector( '#editor' ), {
        plugins: [ Link, Typing, Paragraph, Clipboard, Undo, Enter ],
        toolbar: [ 'link', 'undo', 'redo' ],
        link: {
            decorators: {
                isExternal: {
                    mode: 'manual',
                    label: 'Open in a new tab',
                    attributes: {
                        target: '_blank'
                    }
                }
            }
        }
    } );

More info will be available in the docs.

Sample screencast: screencast showing open in a new tab switch added to the link panel

yura-tovt commented 5 years ago

Hi @mlewand. Thanks for implementing this feature. But it seems that upcast conversion for additional attributes is missing.

msamsel commented 5 years ago

Hi @mlewand. Thanks for implementing this feature. But it seems that upcast conversion for additional attributes is missing.

I'm on it. As I talk with @Reinmar it's important issue. It's reported here as well ckeditor/ckeditor5-link#233.

mlewand commented 5 years ago

Thanks for catching that! We're working hard to include it in the upcoming release, keep an eye on ckeditor/ckeditor5-link#233 for quickest update.

Nivekul commented 5 years ago

I followed this tutorial and added the link: { addTargetToExternalLinks: true } configuration. But it did absolutely nothing, is there something I'm missing?

I'm using CKEditor5 Classic Build in Vue, version 12.3.1.

oleq commented 5 years ago

@Nivekul What data (editor content) did you use?

Nivekul commented 5 years ago

@Nivekul What data (editor content) did you use?

@oleq I'm not sure what is the data (editor content), but the following is how I setup the editor

Template

<ckeditor v-model="article" class="editor" :editor="editor" :config="editorConfig"
          @focus="editorFocused=true" @input="handleEdit"></ckeditor>

Data

article: '',
editor: ClassicEditor,
editorConfig: {
    toolbar: {
        items: [
            'Heading', '|',
            'bold', 'italic',
            'link', 'blockQuote',
            'ImageUpload',
            'MediaEmbed',
        ]
    },
    link: {
        addTargetToExternalLinks: true,
    },
    image: {
        toolbar: ['imageStyle:alignLeft', 'imageStyle:full', 'imageStyle:alignRight', '|', 'imageTextAlternative'],
        styles: ['full', 'alignLeft', 'alignRight'],
    },
    placeholder: 'Start writing your article here ...',
    extraPlugins: [(editor) => this.uploadAdapter(editor, this)],
},
oleq commented 5 years ago

So what happens to links starting with "https://" or "http://" or "//" in your content?

Nivekul commented 5 years ago

@oleq I see, it works now! Thanks!

zehawki commented 4 years ago

I felt compelled to come here and add a note that this feature is the cat's whiskers :-) What a fricking lifesaver to be able to put an auto rule that sends external links to a new window without having to worry about writers screwing the UX for readers. Thank you! CK editor is lovely, and thank you for listening to the community.

sdavid501 commented 3 years ago

link: { defaultProtocol: 'http://', addTargetToExternalLinks: true }

Madejczyk commented 3 years ago

@mlewand do you have somewhere branch with your changes mentioned here: https://github.com/ckeditor/ckeditor5/issues/4829#issuecomment-505882498 I would like to check config of decorators.

mlewand commented 2 years ago

@Madejczyk I haven't set up any particular branch containing this configuration. All that is required is just some simple config adjustment.

I made a demo so you can play with it: https://codepen.io/mlewand/pen/NWvooJg?editors=1010

MrBeeUser commented 1 year ago
link: {
            decorators: {
                openInNewTab: {
                    mode: 'manual',
                    label: 'Open in a new tab',
                    attributes: {
                        target: '_blank',
                        rel: 'noopener noreferrer'
                    }
                }
            }
        }

See here