search

mettke / minify-rs

Crate for text minification. Currently supported: html, json
MIT License
1 stars 0 forks source link

Whitespace removed in inline formatting context #1

Closed vallentin closed 3 years ago

vallentin commented 3 years ago

Whitespace seems to be removed incorrectly between some inline elements.

Minimal, Reproducible Example

let html = "<p>Foo  <span>Bar</span>  \n\n  <span>Baz</span></p>";
let html = minify::html::minify(html);
println!("{:?}", html);

Expected:

_Based on MDN._

"<p>Foo <span>Bar</span> <span>Baz</span></p>"

Actual:

"<p>Foo<span>Bar</span><span>Baz</span></p>"

Which makes it visually render as "FooBarBaz" instead of "Foo Bar Baz".

mettke commented 3 years ago

Thanks for the report. This should be fixed with Version 1.2.0. Feel free to reopen if the issue remains.

vallentin commented 3 years ago

I must say, given that the last release was 2 years ago. I'm quite impressed and thankful, that you fixed it within just hours! Thanks a lot!

The new release does indeed (visually) render correctly now. For constructive feedback, too much whitespace remains in some cases. But honestly, I'd rather have too much whitespace than too little.

mettke commented 3 years ago

Yeah that's unfortunately correct. However, it is very hard to detect whether a given white space should be removed or not. If you have idea's feel free to share and I'll think about it.

One idea would be to remove white spaces between > and < but even that would be wrong in corner cases like <b>test</b> <b>test</b>. So for now I think this is the best solution.

vallentin commented 3 years ago

I have one idea for a specific case, though note that I didn't look at your implementation, so I don't really know if it's feasible to do.

If you know whether you're inside the <head> vs <body>. Then if inside the <head> you wouldn't need to retain all whitespace.