Skip to content

Heading levels are messed up in the reference page of SecurityBundle #20716

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.

Sign up for GitHub

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

Jump to bottom
Closed
stof opened this issue Mar 4, 2025 · 3 comments
Closed

Heading levels are messed up in the reference page of SecurityBundle #20716

stof opened this issue Mar 4, 2025 · 3 comments
Labels
hasPR A Pull Request has already been submitted for this issue.

Comments

@stof
Copy link
Member

stof commented Mar 4, 2025

https://symfony.com/doc/current/reference/configuration/security.html has a messed up heading hierarchy.

access_control and firewalls are top-level keys, not subkeys of session_fixation_strategy. And all headings coming after firewalls should be sub-heading of firewalls as they document its subkeys. role_hierarchy and providers are also top-level jets but are documented as subkeys of required_badges (which is should be one of the firewalls subkeys)

some other things also don't make sense at all. delete_cookies is a sub-key of logout but is not documented as such (fortunately, there is a code snippet).
@stof
Copy link
Member Author

stof commented Mar 4, 2025

Note that the reference is also incomplete. For instance, the top-level key access_decision_manager does not appear anywhere in the page. And password_hashers does not appear either.

@javiereguiluz javiereguiluz removed their assignment Mar 5, 2025
@javiereguiluz
Copy link
Member

@OskarStark I'm removing myself from being assigned in this issue. I'm sorry but I don't understand the Symfony Security component well, so I can't work on this.

mmauksch added a commit to mmauksch/symfony-docs that referenced this issue Mar 5, 2025
@mmauksch
Fix Headings mentioned in symfony#20716
cb48aa5
@mmauksch
Copy link
Contributor

mmauksch commented Mar 5, 2025

Fixing the headings was a quick fix.

In general that site might need some love in general though. I think moving some of the links mentioned in basic options to the relevant subheadings to give these links more context. My first day contributing to symfony docs though so I would prefer to read up a bit more on how these pages should be structured before I do any bigger changes.

OskarStark pushed a commit to mmauksch/symfony-docs that referenced this issue Mar 6, 2025
@mmauksch @OskarStark
Fix Headings mentioned in symfony#20716
f235bc6
OskarStark added a commit that referenced this issue Mar 6, 2025
@OskarStark
minor #20727 Fix Headings mentioned in #20716 (mmauksch)
36280dd 
This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

Fix Headings mentioned in #20716

Changed headings so the sidebar won't be formatted wrong

Commits
-------

f235bc6 Fix Headings mentioned in #20716
@OskarStark OskarStark mentioned this issue Mar 6, 2025
Merged
OskarStark added a commit to OskarStark/symfony-docs that referenced this issue Mar 6, 2025
@OskarStark
Merge branch '6.4' into 7.2
5a775a4 
* 6.4:
  Fix Headings mentioned in symfony#20716
OskarStark added a commit to OskarStark/symfony-docs that referenced this issue Mar 6, 2025
@OskarStark
Merge branch '7.2' into 7.3
0473a80 
* 7.2:
  Fix Headings mentioned in symfony#20716
@stof stof mentioned this issue Mar 6, 2025
Merged
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this issue Mar 6, 2025
@javiereguiluz
Merge branch '7.2' into 7.3
8788120 
* 7.2:
  Fix Headings mentioned in symfony#20716
  fix: Add missing ContainerBuilder use statement in Bundle documentation
  Update overview.rst
  Add missing default value in the Twig reference
javiereguiluz added a commit that referenced this issue Mar 6, 2025
@javiereguiluz
minor #20729 [Security] Fix more heading levels in the security refer…
a3bf885 
…ence (stof)

This PR was merged into the 6.4 branch.

Discussion
----------

[Security] Fix more heading levels in the security reference

This does the remaining fixes for #20716:

- ensure that firewall subkeys are indeed nested
- remove the documentation of `delete_cookies` as a `<h2>` (we already have the duplicate content in the appropriate location under `logout`)
- fix `providers` and `role_hierarchy` being rendered properly as h2 (this is actually fix by changing the delimiter used for h2, being consistent with the other documentation pages, keeping the `~~~` delimiter for elements that should become `h3`).

Commits
-------

cb7e538 Fix more heading levels in the security reference
@xabbuh xabbuh added the hasPR A Pull Request has already been submitted for this issue. label Mar 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@javiereguiluz @stof @OskarStark @xabbuh @mmauksch