-
Notifications
You must be signed in to change notification settings - Fork 289
Chore: Remove invalid closing
and
, self-closing slashes, xml:lang
and xmlns
#4386
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
base: main
Are you sure you want to change the base?
Conversation
While the self-closing slash may also be unnecessary, at least it's valid
✅ Deploy Preview for wcag2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Assume we could expand this to also remove the |
with self-closing slash
and
with self-closing slash
The Also, I would point out that the build system is fixing these (without the self-closing tag). If I look for these instances in the live pages, I don't see them in the code that is output. So I'm wondering if this isn't worth the effort (and potential for conflicts) to merge, somewhat akin to #3927 which we closed last week after your suggestion. |
Oh yeah. I've been wanting to yoink them for ages but wasn't sure if we were keeping them around for some reason. |
7675e97
to
41b5428
Compare
oops, fat finger typo. I corrected it and force-pushed Regarding potential for conflicts: I think this sort of change should be fine to make, as it would only collide in cases where a PR already made some changes to those bits of the pages... |
and
with self-closing slash
and
, self-closing slashes, xml:lang
and xmlns
Expanded this PR to remove other XHTML-style rubbish that's now unnecessary (as we're not bound to the XSLT pipeline to build the documents). Controversial perhaps, but: although this crud may be getting automagically removed at actual build time, it would be nice to just rip this plaster off swiftly once and decrap at source. Again, this should not present a big risk of merge conflicts - possibly just a handful of manual tweaks needed for any PRs currently in flight if they affected any images where we're now removing the self-closing slash, for instance |
9298813
to
b36b91a
Compare
b36b91a
to
9eb5e5d
Compare
I have spent the past ~45 minutes reviewing these changes to check for any unintentional changes, especially in the self-closing tag cleanup. (Thanks Francis for spotting the MathML instances.) I didn't find anything else unintentionally caught in that crossfire, but I did find some more I would point out this is changing files we typically have no interest in changing, so I'm wondering how we feel about them being included in this PR:
If we want to back any of these out, I can do so quickly. (Update 2025-05-30: We agreed on the call to back out these files, so I've added a commit to the branch that does so.) In addition, note this touches |
and
, self-closing slashes, xml:lang
and xmlns
and
, self-closing slashes, xml:lang
and xmlns
just a gentle nudge to not let this sit for too long, as the longer it's in this state, the more likely that it will start diverging from main in ways that aren't always easy to recover from... |
thank you for the assist @kfranqueiro ... think that's now ok to do/send to AGWG/whatever |
@kfranqueiro I believe there were a few files you were going to remove or tweak before putting in front of working group? |
Removes all sorts of unnecessary XHTML cruft that we kept carrying around due to the old XSLT-based publication pipeline which is now obsolete