feat: auto rel=noopener on target=_blank links (configurable)#9
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6f1310792
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sig { params(node: Nokogiri::XML::Node).returns(String) } | ||
| def target_attribute(node) | ||
| node.attributes['target'] ? %( target="#{escape_attr(node.attributes['target'])}") : '' | ||
| def link_attributes(node) |
There was a problem hiding this comment.
README.md:285-287 documents target_attribute as an available Base helper for custom components, so any existing component that calls it from #transform will now raise NoMethodError as soon as it renders after upgrading to this 1.2.0 change. Since link_attributes is a superset, keep a target_attribute wrapper/alias for compatibility rather than removing the documented helper outright.
Useful? React with 👍 / 👎.
| def link_attributes(node) | ||
| target = node.attributes['target']&.value | ||
| rel = node.attributes['rel']&.value | ||
| rel ||= ActiveMail.configuration.blank_link_rel if target == '_blank' |
There was a problem hiding this comment.
When an author writes target="_BLANK" or another mixed-case spelling, the link still targets a blank browsing context, but this exact comparison skips blank_link_rel and emits no rel, leaving the new reverse-tabnabbing hardening disabled for that input. Compare the target case-insensitively before deciding whether to inject the configured rel value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c44bc48d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| IGNORED_ON_PASSTHROUGH = T.let( | ||
| %w[class id href size large no-expander small target up size-sm size-lg style].freeze, | ||
| %w[class id href size large no-expander small target rel up size-sm size-lg style].freeze, |
There was a problem hiding this comment.
For custom components that use the documented pass_through_attributes helper to build their own anchors, e.g. emitting <a #{pass_through_attributes(node)}>, a source attribute like rel="nofollow" is now stripped globally and that custom component never gets the built-in link_attributes path that re-emits it. This silently drops author-supplied link semantics/security outside the three built-in link components; keep rel out of the generic passthrough ignore list or make the extra filtering local to the built-in anchor emitters.
Useful? React with 👍 / 👎.
1 participant
What
Anchors with
target="_blank"(<cta>,<button>,<item>) now emit arelautomatically, hardening links against reverse-tabnabbing.ActiveMail.config.blank_link_rel(default"noopener"); setnilto disable.rel="…"on the tag always wins and is never duplicated (reladded toIGNORED_ON_PASSTHROUGH).Base#link_attributes(renamed fromtarget_attribute), shared by all anchor components — no per-project template edits.Why
Generic, consumer-agnostic default: security without stripping the referrer (analytics-safe). Each project overrides via config.
Tests
Component tests (cta/button/menu) + configuration tests: rel on
_blank, absent otherwise, config override,nilopt-out, explicit-rel precedence without duplication.rake test/srb tc/rubocopgreen.