-
Notifications
You must be signed in to change notification settings - Fork 153
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
Conversation
There was a problem hiding this 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.
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.
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.
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).Bad value origin-trial for attribute http-equiv
false positive with validator https://github.com/whatwg/html/issues/7858
source/js/components/newsletter-signup/organisms/default-layout-signup.jsx
Show resolved
Hide resolved
There was a problem hiding this 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
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
Checklist
Tests
Changes in Models:
Documentation:
Merge Method
💡❗Remember to use squash merge when merging non-feature branches into
main