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

Parsing an attribute without a value can cause code confusion #486

Closed
Bulandent opened this issue Mar 24, 2023 · 3 comments · Fixed by #485
Closed

Parsing an attribute without a value can cause code confusion #486

Bulandent opened this issue Mar 24, 2023 · 3 comments · Fixed by #485
Labels
bug Something isn't working xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed

Comments

@Bulandent
Copy link
Contributor

Describe the bug

Something went wrong when I parsed the following code。

<template>
  <view>
    <image lazy />
    <image></image>
  </view>
</template>

The above code is parsed into something like this:

<template>
  <view>
    <image lazy="lazy">
    <image/>
  
</image></view></template>

To Reproduce
https://stackblitz.com/edit/js-xmldom-template-cyojup?file=index.js

Expected behavior

<template>
  <view>
    <image lazy="lazy"/>
    <image/>
  </view>
</template>

Runtime & Version:
xmldom version: 0.8.6
runtime version: 0.8.6
other related software and version:

Additional context

I tried to solve the problem myself, until I changed the code in @xmldom/xmldom/lib/sax.js in such a way that it could parse properly。

case '/':
	switch(s){
	case S_TAG:
		el.setTagName(source.slice(start,p));
	case S_ATTR_END:
	case S_TAG_SPACE:
	case S_TAG_CLOSE:
		s =S_TAG_CLOSE;
		el.closed = true;
	case S_ATTR_NOQUOT_VALUE:
	case S_ATTR:
-	// case S_ATTR_SPACE:
		break;
+	case S_ATTR_SPACE:
+		el.closed = true;
+		break;
	//case S_EQ:
	default:
		throw new Error("attribute invalid close char('/')") // No known test case
	}
	break;

Finally, I put up the pull request, I don't know whether my solution is right or not, so you can check it when you are free.
Thanks.

@Bulandent Bulandent added bug Something isn't working needs investigation Information is missing and needs to be researched labels Mar 24, 2023
@karfau
Copy link
Member

karfau commented Mar 24, 2023

Thank you for reporting, filing the bug and even the PR.
Let's see how the tests think about those changes.

karfau added a commit that referenced this issue Mar 31, 2023
Closes #486

---------

Co-authored-by: chengcp <ccp06939@itsmycar.cn>
Co-authored-by: Christian Bewernitz <coder@karfau.de>
@karfau
Copy link
Member

karfau commented Mar 31, 2023

Fixes are available in 0.7.10 and 0.8.7 releases
(Since I'm still working on something for the upcoming next release 0.9.0-beta.7, this fix will only be included once that version is released.)

@karfau karfau added xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed and removed needs investigation Information is missing and needs to be researched labels Mar 31, 2023
@karfau
Copy link
Member

karfau commented Mar 31, 2023

FYI: Just added the xml:not well-formed label since this is only valid HTML, not XML according to the specs and the DOMParser in browser will refuse to parse all of it:

image

Even when using text/html as a mimeType the behavior is different in the browser (where template tags end up in the head and it's content is inside a DocumentFragment):

image

compared to xmldom, which doesn't wrap html with a html, body and headand doesn't have any special handling of the template tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants