Skip to content

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

patrickhlauke
Copy link
Member

@patrickhlauke patrickhlauke commented May 7, 2025

Removes all sorts of unnecessary XHTML cruft that we kept carrying around due to the old XSLT-based publication pipeline which is now obsolete

While the self-closing slash may also be unnecessary, at least it's valid
Copy link

netlify bot commented May 7, 2025

Deploy Preview for wcag2 ready!

Name Link
🔨 Latest commit 8311ef2
🔍 Latest deploy log https://app.netlify.com/projects/wcag2/deploys/6839d4113f814a0008dc7f86
😎 Deploy Preview https://deploy-preview-4386--wcag2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@patrickhlauke
Copy link
Member Author

Assume we could expand this to also remove the xml:lang="en" xmlns="http://www.w3.org/1999/xhtml"

@patrickhlauke patrickhlauke changed the title Replace invalid closing with self-closing slash Replace invalid closing and with self-closing slash May 7, 2025
@kfranqueiro
Copy link
Contributor

The link tags look like they weren't modified correctly; there is now a doubled closing bracket in every link tag.

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.

@fstrr
Copy link
Contributor

fstrr commented May 8, 2025

Assume we could expand this to also remove the xml:lang="en" xmlns="http://www.w3.org/1999/xhtml"

Oh yeah. I've been wanting to yoink them for ages but wasn't sure if we were keeping them around for some reason.

@patrickhlauke patrickhlauke force-pushed the patrickhlauke-remove-closing-meta branch from 7675e97 to 41b5428 Compare May 8, 2025 20:22
@patrickhlauke
Copy link
Member Author

The link tags look like they weren't modified correctly; there is now a doubled closing bracket in every link tag.

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...

@patrickhlauke patrickhlauke changed the title Replace invalid closing and with self-closing slash Remove invalid closing and , self-closing slashes, xml:lang and xmlns May 9, 2025
@patrickhlauke
Copy link
Member Author

patrickhlauke commented May 9, 2025

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

@patrickhlauke patrickhlauke force-pushed the patrickhlauke-remove-closing-meta branch from 9298813 to b36b91a Compare May 9, 2025 10:53
@patrickhlauke patrickhlauke force-pushed the patrickhlauke-remove-closing-meta branch from b36b91a to 9eb5e5d Compare May 9, 2025 10:55
@kfranqueiro
Copy link
Contributor

kfranqueiro commented May 9, 2025

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 tags to clean up.

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:

  • conformance-challenges/index.html
  • requirements/21/index.html and requirements/22/index.html
  • Many files under wcag20/ which we generally avoid touching; I would particularly consider backing out changes here, as I am unsure if whatever build process they were involved in requires XHTML

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 guidelines/index.html and guidelines/relative-luminance.html, though not in ways that are at all consequential to the written content.

@patrickhlauke patrickhlauke changed the title Remove invalid closing and , self-closing slashes, xml:lang and xmlns Chore: Remove invalid closing and , self-closing slashes, xml:lang and xmlns May 27, 2025
@patrickhlauke
Copy link
Member Author

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...

@patrickhlauke
Copy link
Member Author

thank you for the assist @kfranqueiro ... think that's now ok to do/send to AGWG/whatever

@mbgower
Copy link
Contributor

mbgower commented Jun 6, 2025

@kfranqueiro I believe there were a few files you were going to remove or tweak before putting in front of working group?

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.

4 participants