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

Fix - Validation errors on default page #11867

Merged
merged 3 commits into from
Feb 16, 2024

Conversation

Morsey187
Copy link
Contributor

Description

Link to sample test page: /en/artifacts/x-ray-goggles/
Related PRs/issues: #11646

The changes in this PR aim to solve the following HTML validation errors

Screenshot 2024-02-14 at 06-12-22 Showing results for contents of text-input area - Nu Html Checker

Checklist

Tests

  • Is the code I'm adding covered by tests?

Changes in Models:

  • Did I update or add new fake data?
  • Did I squash my migration?
  • Are my changes backward-compatible. If not, did I schedule a deploy with the rest of the team?

Documentation:

  • Is my code documented?
  • Did I update the READMEs or wagtail documentation?

Merge Method
💡❗Remember to use squash merge when merging non-feature branches into main

Copy link
Contributor Author

@Morsey187 Morsey187 left a comment

Choose a reason for hiding this comment

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

Notes on issues not addressed in this PR but highlighted in the validator.

Stray start tag script.

The script tag needs to be inside the body tag.

Initially I thought this was related to the code injecting the widget script, however it's the scripts being injected by the widget script itself, meaning there isn't much we can do this about apart from raise an issue with the provider along the lines of

The following scripts being injected by the widget script should be injected inside the body tag not outside of it.

<script type="text/javascript" async="" src="https://static.fundraiseup.com/401ad27e677a.elementsApi.js"></script>
<script type="text/javascript" async="" src="https://static.fundraiseup.com/checkout.4a5eff42260724f4160e.js"></script>
<script type="text/javascript" async="" src="https://static.fundraiseup.com/checkout-vendors.c58302b7a0e16ce928c0.js"></script>
<script type="text/javascript" async="" src="https://static.fundraiseup.com/checkout-styles.1e7407780ae7ba319eea.js"></script>

However, it's not a huge problem as most issues with HTML validation are related to the intial server side rendered markup sent over the network, this is injected after rendering.

Screenshot 2024-02-15 at 10 22 50
The charset attribute on the script element is obsolete.

I can remove this, however I wouldn’t think there’s much point as we’d only be weakening legacy browser support and we’d still have these same validation errors from scripts injected by a few of the third-party resources.

Screenshot 2024-02-15 at 10 21 42
Unnecessary type attribute Same as the above around the charset attribute.

I can remove this, however I wouldn’t think there’s much point as we’d only be weakening legacy browser support and we’d still have these same validation errors from scripts injected by a few of the third-party resources.

Screenshot 2024-02-15 at 10 21 30
charset meta element False positive, issue is with the position of the charset meta element. `<style id="__funBackdropStyles">` is injected above `` so can't do much about this one, however not a major issue as it's not part of the initally rendered markup (I must have copied the markup after the DOM loaded on this one). Screenshot 2024-02-15 at 10 21 24
Bad value origin-trial for attribute http-equiv false positive with validator https://github.com/whatwg/html/issues/7858 Screenshot 2024-02-15 at 10 21 36

@Morsey187 Morsey187 requested a review from mmmavis February 15, 2024 12:40
@Morsey187 Morsey187 changed the title Fix validation errors Fix - Validation errors on default page Feb 15, 2024
Copy link
Collaborator

@mmmavis mmmavis left a comment

Choose a reason for hiding this comment

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

✨ Thanks for addressing these errors @Morsey187

@mmmavis mmmavis enabled auto-merge (squash) February 16, 2024 00:13
@mmmavis mmmavis merged commit e2a8c14 into main Feb 16, 2024
8 checks passed
@mmmavis mmmavis deleted the fix/11646-HTML-validation-errors-default-page branch February 16, 2024 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants