Skip to content

Include button name/value in form submissions. Full application/xhtml+xml support.#8

Merged
Amaury merged 2 commits intoDigicreon:pr8/xhtml-submitterfrom
nullterminated:main
Mar 17, 2026
Merged

Include button name/value in form submissions. Full application/xhtml+xml support.#8
Amaury merged 2 commits intoDigicreon:pr8/xhtml-submitterfrom
nullterminated:main

Conversation

@nullterminated
Copy link
Contributor

@nullterminated nullterminated commented Mar 16, 2026

Hi again,

I have a couple more bug fixes if you want them.

  1. compare all tagName using toUpperCase except body. replaceWith() works on body elements in application/xhtml+xml and should be used there.

  2. include the submit button's name/value on the submitted form data.

@Amaury Amaury changed the base branch from main to pr8/xhtml-submitter March 17, 2026 10:18
@Amaury Amaury merged commit b026081 into Digicreon:pr8/xhtml-submitter Mar 17, 2026
Amaury added a commit that referenced this pull request Mar 17, 2026
XHTML tagName normalization is only needed in _elKey() and _mergeHead(),
which compare elements across documents. All other tagName checks operate
within the same parsing context. Fetched responses are always parsed with
DOMParser as text/html, producing uppercase tagNames regardless of the
server's Content-Type.
Amaury added a commit that referenced this pull request Mar 17, 2026
Rewrite the submitter handling from PR #8 for backward compatibility.
The original code used new FormData(form, e.submitter) which requires
Chrome 111+. The new approach tracks the last clicked submit button
via _onClick and falls back to it when e.submitter is not available.
@Amaury
Copy link
Contributor

Amaury commented Mar 17, 2026

Thanks for this PR! The submit button name/value issue is indeed a real bug, good catch.

I've merged your PR and made two follow-up changes on top of it:

Regarding toUpperCase(): I've reverted the tagName normalization additions, keeping only the two that were already in place (_elKey() and _mergeHead()). µJS parses all fetched responses with DOMParser.parseFromString(html, "text/html"), which always produces uppercase tagName values. The only case mismatch can occur when comparing elements from the current document against elements from the parsed response, which is exactly what _elKey() and _mergeHead() do. All other tagName checks compare elements within the same document, so their casing is always consistent.

Regarding the submit button: I've rewritten the FormData handling for backward compatibility. new FormData(form, submitter) requires Chrome 111+, Firefox 111+, and Safari 16.4+, which is above µJS's compatibility target. The new approach tracks the last clicked submit button in _onClick and uses it as a fallback when e.submitter is not available. The end result is the same (the submit button's name/value is included in the request) but it works on older browsers too.

@Amaury Amaury mentioned this pull request Mar 17, 2026
@nullterminated
Copy link
Contributor Author

nullterminated commented Mar 17, 2026

Cute. Claude told me I'm the one hallucinating. It would be one thing to disagree with my proposed solution, but it's an entirely different thing to say the bug I fixed simply doesn't exist and throw out the patch.

I just retested with the latest version on jsdelivr. The form submission issue I mentioned in #5 still exists thanks to the removal of the toUpperCase() calls.

@Amaury
Copy link
Contributor

Amaury commented Mar 18, 2026

I'm sorry you feel that way. I genuinely spent time analyzing both your bug report and your PR, and I appreciate your involvement.

A few things I'd like to clarify:

About the XHTML issue: In #5, the reproduction context wasn't provided (Content-Type, doctype, etc.), which made it difficult to reproduce the bug on my end. µJS parses all fetched responses with DOMParser.parseFromString(html, "text/html"), which always produces uppercase tagName values regardless of the server's Content-Type. This means a case mismatch can only occur when comparing elements from the current document against elements from the parsed response, which is what _elKey() and _mergeHead() do. These two functions already had toUpperCase() applied before your PR, and they are the only code paths where cross-document comparison happens. The other tagName checks operate on elements within the same document, where casing is always consistent.

Beyond the technical argument, true XHTML served as application/xhtml+xml represents a very small fraction of the web today. µJS is a ~5KB library where every byte counts. Adding toUpperCase() to every tagName comparison throughout the codebase to support an edge case that virtually no one encounters is not a trade-off I'm willing to make.

That said, if you're still experiencing the issue with the latest version, I'd be happy to investigate further, but I'll need specific reproduction steps: the Content-Type your server returns, a minimal HTML example, and the browser you're testing with.

As a practical suggestion: if you're using XHTML mainly for the benefit of XML validation, you can serve your pages with Content-Type: text/html instead of application/xhtml+xml. The browser will parse them in HTML mode while your markup remains XML-valid. This is a common practice and would sidestep the tagName casing issue entirely.

About the PR structure: Your PR contained two unrelated changes (XHTML normalization and submit button handling). The polite and constructive approach would have been to submit two separate PRs, one per concern. It makes review easier, and avoids the situation we're in now where one part is accepted and the other isn't, which can feel frustrating for both sides.

About the submit button fix: This is a real bug, and I'm grateful you caught it. I rewrote the implementation for backward compatibility, because new FormData(form, submitter) requires Chrome 111+, Firefox 111+, and Safari 16.4+, which is above µJS's compatibility target. The new approach uses a click-tracking fallback that achieves the same result on older browsers. I also chose to merge your PR (rather than cherry-pick or rewrite from scratch) specifically so that you'd appear as a contributor to the project. That felt right given your involvement.

About disagreeing with a solution: Reviewing a PR and deciding not to include part of it is a normal part of the process. It doesn't mean the bug doesn't exist; it means I'm not convinced the proposed fix is necessary given how the library works internally. If I'm wrong, a concrete reproduction case will help me understand what I'm missing.

@nullterminated
Copy link
Contributor Author

You said,

µJS parses all fetched responses with DOMParser.parseFromString(html, "text/html"), which always produces uppercase tagName values. The only case mismatch can occur when comparing elements from the current document against elements from the parsed response, which is exactly what _elKey() and _mergeHead() do. All other tagName checks compare elements within the same document, so their casing is always consistent.

Which is clearly wrong. Now you say

µJS is a ~5KB library where every byte counts. Adding toUpperCase() to every tagName comparison throughout the codebase to support an edge case that virtually no one encounters is not a trade-off I'm willing to make.

Which is to my earlier point. If you don't like the solution, that's fine. mujs has features I'll never use and I can save more kilobytes by removing those. I'll just fork off and stay out of your way. Good luck with your project.

@Amaury
Copy link
Contributor

Amaury commented Mar 18, 2026

When you say "which is clearly wrong", you're not providing any factual evidence. No Content-Type header, no minimal reproduction case, no browser name, no error message. You're asking me to take your word for it and merge code without question. That's not how open-source collaboration works. I've asked multiple times for concrete reproduction steps; the offer still stands.

As for forking, that's exactly what open-source licenses are for. I wish you the best of luck with your project.

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