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

HTML hyphenation leads to broken HTML markup #10

Closed
dlubitz opened this issue Feb 28, 2023 · 1 comment · Fixed by #11
Closed

HTML hyphenation leads to broken HTML markup #10

dlubitz opened this issue Feb 28, 2023 · 1 comment · Fixed by #11
Assignees
Labels
bug Something isn't working

Comments

@dlubitz
Copy link
Contributor

dlubitz commented Feb 28, 2023

If you hyphenate an HTML text which has no single root element, the resulting HTML markup is rearranged and broken.

This was introduced with this commit.

It's a known issue of the underlaying XML Lib:
https://stackoverflow.com/questions/29493678/loadhtml-libxml-html-noimplied-on-an-html-fragment-generates-incorrect-tags
https://stackoverflow.com/questions/4879946/how-to-savehtml-of-domdocument-without-html-wrapper

To Reproduce

hyphenate = Carbon.Hyphen:Html {
    content = '<p>foo</p><p>bar</p>'
}

leads to:

<p>foo<p>bar</p></p>

Expected behavior
The resulting markup should stay the same as before the hyphenation.

How to fix
The fix is very easy, as it just need some kind of "wrapper" tag to enfoce the HTML to have a single root element.

<div><p>foo</p><p>bar</p></div>

Question is, if this package should handle that autmatically? Or the package integrator needs to take care of this?

I could image to wrap the text internally with a div and remove the wrapper afterwards:
https://github.com/CarbonPackages/Carbon.Hyphen/blob/main/Classes/Fusion/Implementation.php#L120-L121

WDYT? @jonnitto
If this works for you, I could prepare a PR. Otherwise we should note this somewhere in the docs.

@dlubitz dlubitz added the bug Something isn't working label Feb 28, 2023
@jonnitto
Copy link
Member

Thanks for your findings! A PR is very welcome!

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

Successfully merging a pull request may close this issue.

2 participants