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

Prevent user error when incorrectly formatted footnotes are added to HTML attachments #287

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

AshGDS
Copy link
Contributor

@AshGDS AshGDS commented Aug 30, 2023

When incorrectly formatted footnotes are used on HTML attachments in Whitehall, the function would cause an unhelpful user error to be displayed.

html.gsub! seemed to crash with footnotes in an attachment, as html was nil: https://govuk.sentry.io/issues/4415164546/?project=202259&query=is%3Aunresolved&referrer=issue-stream&stream_index=0

This will allow the attachment to save.

This repo is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.

@AshGDS AshGDS changed the title Prevent user error when invalid footnotes are added to HTML attachments Prevent user error when footnotes are added to HTML attachments Aug 30, 2023
@AshGDS AshGDS force-pushed the acronym_error_fix branch 2 times, most recently from 2bd1f40 to 539b6c5 Compare August 30, 2023 15:17
Copy link
Member

@brucebolt brucebolt left a comment

Choose a reason for hiding this comment

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

The code change looks good 👍.

I've made a suggestion for the CHANGELOG, to clarify that this only affects incorrectly formatted footnotes.

I'd also suggest adding an example of the incorrect formatting we saw in this document into the commit message, so there's a permanent record of the problem being solved here.

CHANGELOG.md Outdated
@@ -1,3 +1,7 @@
## Unreleased

* Prevent user error when footnotes are added to HTML attachments ([#287](https://github.com/alphagov/govspeak/pull/287))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Prevent user error when footnotes are added to HTML attachments ([#287](https://github.com/alphagov/govspeak/pull/287))
* Prevent user error when incorrectly formatted footnotes are added to HTML attachments ([#287](https://github.com/alphagov/govspeak/pull/287))

Copy link
Contributor Author

@AshGDS AshGDS Aug 30, 2023

Choose a reason for hiding this comment

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

Thanks @brucebolt - I removed the reference to incorrect formatting as I was still getting issues when I removed the example we found. But now I've realised the footnote above the one we identified was broken as well, as it contained | which I think was triggering the table formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll reintroduce the wording you suggested

@AshGDS AshGDS changed the title Prevent user error when footnotes are added to HTML attachments Prevent user error when incorrectly formatted footnotes are added to HTML attachments Aug 30, 2023
…HTML attachments

When footnotes are used in HTML attachments, this function would cause an unhelpful user error to be displayed. This will allow the attachment to save.

Example of incorrectly formatted footnotes:

[^22]: CDC. Centre for Disease Control and Prevention. Invasive Mold Infections in Immunocompromised People [updated 06/06/2019]. Available from: Invasive Mold Infections in Immunocompromised People | Mold | CDC
23. David J. Pevalin, Aaron Reeves, Emma Baker, Rebecca Bentley, The impact of persistent poor housing conditions on mental health: A longitudinal population-based study, Preventive Medicine, Volume 105, 2017, Pages 304-310, https://doi.org/10.1016/j.ypmed.2017.09.020

22 contains the character | which triggers table formatting. 23 is not correctly formatted as a footnote.

Co-authored-by: Bruce Bolt <bruce.bolt@digital.cabinet-office.gov.uk>
@AshGDS AshGDS merged commit 0798a1a into main Aug 30, 2023
5 checks passed
@AshGDS AshGDS deleted the acronym_error_fix branch August 30, 2023 16:10
@AshGDS AshGDS mentioned this pull request Sep 1, 2023
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