Rebelmail/posthtml-minifier

Attribute Pointers Hurt Compression and Meaning

Opened this issue · 2 comments

With no whitelist, given:

<style>
.test {
    padding: 1px;
}
.testspan {
    padding: 2px;
}
.spantest {
    padding: 3px;
}
</style>
<p class="test">
    <span class="testspan"></span>
    <span class="spantest"></span>
</p>

And no whitelist, the results become:

<style>
.zo {
    padding: 1px;
}
.zospan {
    padding: 2px;
}
.spanzo {
    padding: 3px;
}
</style>
<p class=zo>
    <span class=zospan></span>
    <span class=spanzo></span>
</p>

As you can see, once a selector is found that contains any portion of any previously found selector, the latter selector will never be fully compressed. These are called AttributePointers in the source code. This action is intentional, given this test.

I don't know what the thinking was for including this feature, but it has several negative consequences:

  • BEM CSS will tend to remain very large, since only their first section of three sections will be compressed: .btn ... .btn--state-success => .bn ... .bn--state-success
  • Arbitrary sharing of compressed selectors for no meaning, or imply an inappropriate relationship: .laughter ... .slaughterhouse => .ex ... .sexhouse
  • Other long selectors that share any relationship to previous selectors may not be compressed well, and the quality of the compression will get worse the more selectors there are.

For my needs, I simply removed the feature entirely so that every selector will get its own unique compressed selector, rather than to try to find a way to retain the feature. You can see the change on my forked branch. If removing this feature seems like a good idea to you, I can attempt to remove it entirely from the codebase, remove it from the tests, and submit a PR. Otherwise, what was the reasoning on including this feature?

This is the wrong repo (you're in posthtml-minifier), but basically it's because of regex attribute selectors. It's very lazily coded and one day we'll make the algorithm smarter.

Drat! Sorry about that. I got them mixed up, it seems. Should I open an issue on that repo?