Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature propositon: target="_blank" option for external links #681

Closed
sylvain101010 opened this issue May 8, 2019 · 16 comments
Closed

Feature propositon: target="_blank" option for external links #681

sylvain101010 opened this issue May 8, 2019 · 16 comments
Labels
done in pr Already done in a PR enhancement

Comments

@sylvain101010
Copy link
Contributor

Hi,

I would love to have an option which which would trasnform

[Linux](https://fr.wikipedia.org/wiki/Linux)

to

<a href="https://fr.wikipedia.org/wiki/Linux" target="_blank" rel="noopener">Linux</a>

(please note the "noopener" for security reasons).

Instead of needing to write those HTML links manually.

Ideally it could be configurable at the site (config.toml) level or at the file (frontmatter) level.

something like

external_links_target_blank = true

Which would affect only external links.

I'm open to try to implement it if you are okay/busy.

Best regards,
Sylvain Kerkour

@sylvain101010 sylvain101010 changed the title Feature propositon: target="_blank" links option Feature propositon: target="_blank" option for external links May 8, 2019
@Keats
Copy link
Collaborator

Keats commented May 8, 2019

The idea itself makes sense to have at the config.toml level for now. Something to consider is that it is very very likely someone will want to disable that functionality for a specific link and I don't want to add custom syntax to markdown for that.
Is there a precedent in other SSG?

@sylvain101010
Copy link
Contributor Author

As far I know no.

What I propose is:

With the option enabled

[Linux](https://fr.wikipedia.org/wiki/Linux)

Renders to

<a href="https://fr.wikipedia.org/wiki/Linux" target="_blank" rel="noopener">Linux</a>

And we can use standard html links for exception when we don't need to open in a new link

@Keats
Copy link
Collaborator

Keats commented May 11, 2019

It looks like Hugo has it as a markdown parser option: https://gohugo.io/getting-started/configuration/#configure-blackfriday (hrefTargetBlank).

I think adding it as a config.toml field only (for now) is ok

@MichaelByrneAU
Copy link

Hi @Keats, @z0mbie42

I've had a look into what might be required to implement this in the same spirit as Hugo's hrefTargetBlank flag. I guess the first decision is whether this approach aligns with the goals of Zola.

Adding a global option will likely mean that absolute links (even those referring to non-external locations) will be given blank targets. An alternative is to use a custom shortcode for external links which implements the blank target and "noopener" attribute itself - but this obviously puts the burden back on the user.

Overview

If a config.toml flag is the way to go, I imagine the following will be required:

  • Adding a new field to config.rs

  • Modifying the markdown rendering logic in markdown.rs to emit custom HTML for specific link types (see below).

  • Updating appropriate documentation.

Updated Rendering

Zola uses uses pulldown-cmark for its markdown rendering. I believe the only way of emitting the custom HTML necessary for this feature is to directly supply it with that custom HTML.

I'd propose changing the logic in the following section of code so that 'normal' links emit an Html event instead of a normal Link event.

Event::Start(Tag::Link(link_type, link, title)) => {
let fixed_link = match fix_link(link_type, &link, context) {
Ok(fixed_link) => fixed_link,
Err(err) => {
error = Some(err);
return Event::Html("".into());
}
};
Event::Start(Tag::Link(link_type, fixed_link.into(), title))
}

The check for what constitutes a 'normal' link can be found in this function.

Conclusion

Let me know your thoughts on the high-level approach (and even if this is something to be pursued). I'd be happy to implement this in a PR for review if you'd like.

@Keats
Copy link
Collaborator

Keats commented May 18, 2019

Thanks @MichaelByrneAU !
The syntax for internal links is going to change (#686) so it shouldn't be a problem.
I'm still not sure what is the right choice there in terms of configuration, I still need to think about ti.

cc @chris-morgan that might be interested in that

@Keats
Copy link
Collaborator

Keats commented May 26, 2019

See #695 for for another thing to add to that feature.

@Keats
Copy link
Collaborator

Keats commented May 30, 2019

Anyone working on that?

@MichaelByrneAU
Copy link

I can have a look at both this and #695 this weekend. Would you be after an implementation broadly similar to the one I outlined above? (conscious that #686 has been resolved now)

@Keats
Copy link
Collaborator

Keats commented May 31, 2019

I am now wondering if that should be an option of pulldown_cmark rather than Zola since that's a pretty common thing to want from a markdown renderer...

cc @marcusklaas

@marcusklaas
Copy link
Contributor

It may make sense. Can you open an issue on pulldown-cmark? That way we can get more input from other users/ contributors.

@Keats
Copy link
Collaborator

Keats commented May 31, 2019

Done in pulldown-cmark/pulldown-cmark#346

@siddhantgoel
Copy link

I'm not sure what the status here is, but as a workaround I've been using the following JavaScript snippet.

var enableTargetBlank = function() {
    var parents = document.getElementsByClassName('with-target-blank');

    for (let parent of parents) {
        var links = parent.getElementsByTagName('a');

        for (let link of links) {
            if (link.hostname !== window.location.hostname && link.target === "") {
                link.target = "_blank";
            }
        }
    }
};

{% if config.extra.enable_target_blank %}
enableTargetBlank();
{% endif %}

This is added to the base template, and I add the with-target-blank class to the elements where I want the links to open in a new tab. It's a bit of a hack, but it works.

@chris-morgan
Copy link
Contributor

chris-morgan commented Apr 7, 2020

You can actually do this sort of thing in the template to a large extent. Instead of writing {{ page.content }}, use an approach like this:

{{ page.content | replace(from=`<a href="`, to=`<a target="_blank" rel="noopener" href="`)
                | replace(from=`<a target="_blank" rel="noopener" href="` ~ config.base_url, to=`<a href="` ~ config.base_url)
                | replace(from=`<a target="_blank" rel="noopener" href="/`, to=`<a href="/`)
}}

Or perhaps this approach (which may be a shade more robust):

{{ page.content | replace(from=`<a href="` ~ config.base_url ~ `/`, to=`<a  href="` ~ config.base_url ~ `/`)
                | replace(from=`<a href="https://`, to=`<a target="_blank" rel="noopener" href="https://`)
                | replace(from=`<a href="http://`, to=`<a target="_blank" rel="noopener" href="http://`)
}}

With my regular expression replacement filter (see #986), you could make it a shade more robust still. (I’m already doing involved replacements within content; if I wanted this functionality, I’d probably do it that way at present.)

@naps62
Copy link

naps62 commented Jul 27, 2020

@chris-morgan I'm probably missing something (new zola user here), but I'm getting an error when trying out your solution:

Failed to build the site
Error:
* Failed to parse "/home/naps62/projects/naps62.github.io/templates/page.html"
  --> 14:52
   |
14 |         {{ page.content | replace(from=`<a href="` + config.base_url + `/`, to=`<a  href="` + config.base_url + `/`)␊
   |                                                    ^---
   |
   = expected `or`, `and`, `not`, `<=`, `>=`, `<`, `>`, `==`, `!=`, or a filter
D

am I doing something wrong here?

Also, @Keats , what's the status of getting the config option suggested here?

@chris-morgan
Copy link
Contributor

Sorry, I was careless there, + should have been ~.

Also a general note about Tera, its grammar is not as consistent as you might like, so I commonly hit places where you can’t write an expression there and must instead assign the expression to a temporary name with the likes of {% set temp = … %}. I don’t think this is a place where that’s needed, but I say this as general advice.

Keats added a commit that referenced this issue Nov 21, 2020
Keats added a commit that referenced this issue Nov 21, 2020
@Keats Keats added the done in pr Already done in a PR label Nov 27, 2020
@Keats
Copy link
Collaborator

Keats commented Nov 27, 2020

This is implemented in the next branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
done in pr Already done in a PR enhancement
Projects
None yet
Development

No branches or pull requests

7 participants