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

Skip target attribute on links with anchors. #291

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

fnando
Copy link
Contributor

@fnando fnando commented Dec 30, 2024

Links like <a href="#top"> should not receive a target=_blank.

@fnando
Copy link
Contributor Author

fnando commented Dec 30, 2024

@flavorjones should this also ignore links that don't start with http/https? Like, I'd expect /home to open on the same tab, rather than opening a new one.

@flavorjones
Copy link
Owner

@fnando Thank you for this! I totally agree that href anchors should jump within the current page and not load a new tab/window.

I guess, depending on the context, it's possible some users might want same-site links to open in the same tab, too, but it's not obvious to me that that should be the default behavior.

Stepping back for a moment to talk big-picture: Loofah is intended to be a framework that enables people to build their own scrubbers; and many (most?) of the built-in scrubbers are meant to be examples or proofs of concept that users can copy and build upon. So I'm not going to obsess too much on correctness, since it's so easy for users to make their own scrubbers with custom behavior if they want.

@flavorjones
Copy link
Owner

The CI failure is unrelated, hang tight and I'll rebase once it's addressed on main.

@flavorjones flavorjones merged commit 868a852 into flavorjones:main Dec 31, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants