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.

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

move CSRF input field into body #35657

Merged
merged 1 commit into from
Jan 22, 2025
Merged

move CSRF input field into body #35657

merged 1 commit into from
Jan 22, 2025

Conversation

biyeun
Copy link
Member

@biyeun biyeun commented Jan 22, 2025 •
edited
Loading

Product Description

Technical Summary

It is invalid HTML to have an <input /> tag in the <head>, and forces the browser to move all the tags (including stylesheets) following this input tag from the <head> into the <body>. bad things could happen in less forgiving browsers or in the future.

Safety Assurance

Safety story

very safe change. makes code better

Automated test coverage

N/A

QA Plan

Not needed

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@biyeun
move CSRF input field into body
6205a79 
it is invalid HTML to have this be in the HEAD, and forces the browser to put all the resulting tags that should be in HEAD into the BODY.
bad things could happen in less forgiving browsers or in the future.
@biyeun biyeun added the product/invisible Change has no end-user visible impact label Jan 22, 2025
@biyeun biyeun requested review from orangejenny and a team January 22, 2025 17:39
@dimagimon dimagimon added the Risk: Medium Change affects files that have been flagged as medium risk. label Jan 22, 2025
orangejenny
orangejenny approved these changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@orangejenny orangejenny left a comment

Choose a reason for hiding this comment

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

There's this one in the style guide, too:

commcare-hq/corehq/apps/styleguide/templates/styleguide/bootstrap5/base.html

Lines 46 to 48 in 7bb5603

{% block csrf_token_block %}
<input id="csrfTokenContainer" type="hidden" value="{{ csrf_token }}">
{% endblock %}

@biyeun biyeun merged commit f8e57a2 into master Jan 22, 2025
13 checks passed
@biyeun biyeun deleted the bmb/fix-csrf-tag-order branch January 22, 2025 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orangejenny orangejenny orangejenny approved these changes

Assignees
No one assigned
Labels
product/invisible Change has no end-user visible impact Risk: Medium Change affects files that have been flagged as medium risk.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@biyeun @orangejenny @dimagimon