Skip to content

Conversation

wbamberg
Copy link
Collaborator

The "attacks" pages are not consistent about which level to put the "Defense summary checklist" at. I think probably H2 is better so it's accessible from the ToC.

I also added one for clickjacking, as we didn't have one.

@wbamberg wbamberg requested a review from a team as a code owner October 15, 2025 17:42
@wbamberg wbamberg requested review from chrisdavidmills and removed request for a team October 15, 2025 17:42
@github-actions github-actions bot added Content:Security Security docs size/s [PR only] 6-50 LoC changed labels Oct 15, 2025
Copy link
Contributor

github-actions bot commented Oct 15, 2025

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wbamberg the heading level change makes sense to me — good to be consistent, and nice to see these sections in the TOC.

I'm going to approve this, as the next comments are out of scope for this PR, and you might action them in a follow-up PR if you want to, but:

  • The style of these sections is inconsistent. The bulleted lists seem good, but some of them have a lead-up of one or more paragraphs (more of a traditional summary), and some do not.
  • I'm also not sure what "primary defense" and "defense in depth" mean. Is this like "mandatory, definitely do this" and "optional if you want to go further"?
  • You can probably remove the "We can summarize the defenses above as follows:" lines, as the section heading says "summary".

@wbamberg
Copy link
Collaborator Author

@wbamberg the heading level change makes sense to me — good to be consistent, and nice to see these sections in the TOC.

I'm going to approve this, as the next comments are out of scope for this PR, and you might action them in a follow-up PR if you want to, but:

I think these are legit comments and have applied most of them.

  • The style of these sections is inconsistent. The bulleted lists seem good, but some of them have a lead-up of one or more paragraphs (more of a traditional summary), and some do not.

The only one AFAICS that is still different is XS-leaks, and that's tricky because unlike the others it's not a single attack with a single set of defenses, but multiple attacks with multiple overlapping defenses, including some attacks with no real defenses. So I don't really see a good alternative to explaining that.

  • I'm also not sure what "primary defense" and "defense in depth" mean. Is this like "mandatory, definitely do this" and "optional if you want to go further"?

Fair enough. Really I think the message is "do both".

  • You can probably remove the "We can summarize the defenses above as follows:" lines, as the section heading says "summary".

Done, and agreed it is unnecessary.

@chrisdavidmills
Copy link
Contributor

All makes sense, cheers @wbamberg! Nice work.

@chrisdavidmills chrisdavidmills merged commit b07e3b8 into mdn:main Oct 17, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Content:Security Security docs size/s [PR only] 6-50 LoC changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants