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

innerHTML broken for template elements #451

Closed
atk opened this issue Apr 7, 2022 · 5 comments · Fixed by #506
Closed

innerHTML broken for template elements #451

atk opened this issue Apr 7, 2022 · 5 comments · Fixed by #506
Assignees
Labels
bug Something isn't working high priority

Comments

@atk
Copy link

atk commented Apr 7, 2022

Test case:

const t = document.createElement('template');
t.innerHTML = '<div>happy-dom is cool!</div>';
console.log(JSON.stringify(t.innerHTML));

Output in

  • Chrome: "<div>happy-dom is cool!</div>"
  • jsdom: "<div>happy-dom is cool!</div>"
  • linkedom: "<div>happy-dom is cool!</div>"
  • happy-dom: ""

Expectation

happy-dom should behave like every other tested DOM.

Motivation

This breaks the integration into solid-register, which uses a dom on top of node to test solid code fast.

@atk atk changed the title innerHTML not supported for template elements innerHTML broken for template elements Apr 7, 2022
@Mas0nShi
Copy link
Contributor

Mas0nShi commented Apr 8, 2022

get innerHTML from this.childNodes in the Element.ts

public get innerHTML(): string {
const xmlSerializer = new XMLSerializer();
let xml = '';
for (const node of this.childNodes) {
xml += xmlSerializer.serializeToString(node);
}
return xml;
}

but the div element not in this.childNodes.
it's in this.content.childNodes.

public get content(): IDocumentFragment {
if (!this._contentElement) {
this._contentElement = this.ownerDocument.createDocumentFragment();
}
return this._contentElement;
}

so we should overwrite innerHTML like this(maybe error for my understand to sources code):

add code in HTMLTemplateElement.ts

	/**
	 * Returns inner HTML.
	 *
	 * @returns HTML.
	 */
	public get innerHTML(): string {
		const xmlSerializer = new XMLSerializer();
		let xml = '';
		for (const node of this.content.childNodes) {
			xml += xmlSerializer.serializeToString(node);
		}
		return xml;
	}
	/**
	 * Sets inner HTML.
	 *
	 * @param html HTML.
	 */
	public set innerHTML(html: string) {
		super.innerHTML = html;
	}

@capricorn86 Is this a good answer?

@atk
Copy link
Author

atk commented Apr 8, 2022

Thanks. I'll monkey-patch that fix if I detect it to be necessary in my project. Also thanks for maintaining this awesome project! If you want you can close this.

@capricorn86 capricorn86 added bug Something isn't working high priority labels Apr 26, 2022
Mas0nShi added a commit to Mas0nShi/happy-dom that referenced this issue Jun 15, 2022
Mas0nShi added a commit to Mas0nShi/happy-dom that referenced this issue Jun 15, 2022
Mas0nShi added a commit to Mas0nShi/happy-dom that referenced this issue Jun 15, 2022
capricorn86 added a commit that referenced this issue Jun 28, 2022
…html-template

#451 fixes missing inner html in html template
@capricorn86
Copy link
Owner

Thanks to @Mas0nShi we now have support for HTMLTemplateElement.innerHTML 🙂

I will add some additional unit tests as well in a separate PR.

You can read more about the release here:
https://github.com/capricorn86/happy-dom/releases/tag/v5.3.3

@capricorn86
Copy link
Owner

Now when I'm writing more unit tests I have realized that the logic is not complete. outerHTML does not fully work and innerHTML does not work correctly when called in parent elements.

I will re-open this ticket.

@capricorn86 capricorn86 reopened this Jun 28, 2022
@capricorn86 capricorn86 self-assigned this Jun 28, 2022
@capricorn86
Copy link
Owner

We finally have a fix in place that should resolve the remaining issues related to the HTMLTemplateElement 🙂

You can read more about the release here:
https://github.com/capricorn86/happy-dom/releases/tag/v6.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants