search

Swaagie / minimize

Minimize HTML
MIT License
162 stars 18 forks source link

Boolean attributes should only be considered as such on elements that define them #27

Closed ArturDorochowicz closed 9 years ago

ArturDorochowicz commented 9 years ago

With spare: false minimize drops the value of what it considers boolean attributes no matter the element. It should treat boolean attributes as such only on elements that define them.

For example, the test at https://github.com/Moveo/minimize/blob/3c595799006f02ce256f775da9c42a99e6af9aa2/test/minimize-test.js#L136, which checks that attribute disabled is not removed on element h4, should actually not depend on the value of spare option at all, since h4 does not define attribute disabled.

Swaagie commented 9 years ago

I see where your coming from, but the specific example is merely a test case (with bad HTML admittedly). Now I don't mind improving the test so it actually has disabled on an input. But could you provide an example where this actually breaks output? Developers are still required to write valid HTML, minimize will not be able to fix bad HTML.

ArturDorochowicz commented 9 years ago

We spotted the issue at work were we use Angularjs and custom directives. One of them allowed "disabling " which was data bound to expression specified on attribute disabled. Minimize removes the expression leaving just an empty attribute.

Now, of course we can change the name of the attribute used by the directive or enable option spare, but I still think that these attributes should not be touched here. 8 gru 2014 12:51 "Martijn Swaagman" notifications@github.com napisał(a):

I see where your coming from, but the specific example is merely a test case (with bad HTML admittedly). Now I don't mind improving the test so it actually has disabled on an input. But could you provide an example where this actually breaks output? Developers are still required to write valid HTML, minimize will not be able to fix bad HTML.

— Reply to this email directly or view it on GitHub https://github.com/Moveo/minimize/issues/27#issuecomment-66106307.

Swaagie commented 9 years ago

ok so you had something like this <img disabled="disabling"> being turned into <img disabled>? I think you got a point here, the only serious downside I see with this is maintaining all lookup tables per element/groups of elements.

ArturDorochowicz commented 9 years ago

Yes, you got that right. Actually in our case we had an "element" directive, i.e. a new html element, something like <myimg disabled="disabling" />

Swaagie commented 9 years ago

Fixed and released as v1.2.0