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

Report error instead of freezing/hanging #332

Closed
sabrehagen opened this issue Jan 25, 2015 · 20 comments
Closed

Report error instead of freezing/hanging #332

sabrehagen opened this issue Jan 25, 2015 · 20 comments
Labels

Comments

@sabrehagen
Copy link

When a custom element is left unclosed, html-minifier will hang with no error reported. e.g.

<my-elem att="something" </my-elem>

This is different to when a standard html element is left unclosed. e.g.

<div att="something" </div>

which will give a parse error and report the offending statement.

@zackiles
Copy link

zackiles commented Feb 2, 2015

I can confirm this.

@kangax kangax added the feature label Feb 5, 2015
@kangax
Copy link
Owner

kangax commented Feb 5, 2015

Marking as "feature" since it's all part of better error reporting that we'll get to at some point

@kangax
Copy link
Owner

kangax commented Jul 3, 2015

Duplicate of #332

@kangax kangax closed this as completed Jul 3, 2015
@Meligy
Copy link

Meligy commented Jul 4, 2015

@kangax this is not a duplicate of #332, because, um.. this is #332.

@kangax
Copy link
Owner

kangax commented Jul 4, 2015

Oh, whoopsie, sorry :) Too many issues closed...

@kangax kangax reopened this Jul 4, 2015
@hwoarangzk
Copy link

Will there be any update to solve this endless loop error? Really expect it coming soon

@fregante
Copy link
Contributor

fregante commented Jul 4, 2015

@hugowetterberg This is open source. Free. If you expect things to happen, do them yourself… or just fix your HTML, it's more desirable anyway.

@hugowetterberg
Copy link
Contributor

So yeah, @bfred-it, I agree. But I guess that that was a typo and you actually meant @hwoarangzk

@hwoarangzk
Copy link

@bfred-it Maybe you don't know the problem detail. I write the correct html but got endless loop when using html-minifier, and I have to admit that I'm not able to fix it by modifying the html-minifier's code now. That's why I want to know whether this will be fixed in the future

@hugowetterberg
Copy link
Contributor

@hwoarangzk as far as I can see this issue is about the minifier going into an infinite loop in the very specific case of an unclosed (malformed) custom element. So what you have encountered is either off topic for this issue OR your markup is malformed, there isn't really room for a middle ground.

If you're unable to contribute code that's completely ok, there are many kinds of contributions that you can make to open source projects. In this case you should verify that your markup actually is valid or at least doesn't contain the same mistake that this issue is about. Then try to remove markup until you're left with the minimal markup needed to reproduce the bug. Then file a separate issue that documents what you found.

@bfred-it reacted to the tone of your original comment, and rightly so. The details of the problem is simply not relevant as you just don't have any right to demand fixes from your peers. Even if that's not what you intended to do it's what your comment sounded like. Keep in mind that just as others should give you leeway because of the obvious language barriers you should try to be sensitive to when a comment has been misinterpreted. You really can't go wrong with generously apologising for unintended slights and backing up your actual arguments with concrete examples and reasoning.

Good luck and have fun!

@hwoarangzk
Copy link

@hugowetterberg I reported as a new issue #385, but kangax told me that it's duplicated with this issue, so I came in this #332 here. And I said the problem I came across is due to the ambiguous double-quotes even I escaped them. And I don't think that escaped quotes in html is malformed. Another, I'm from China and my English is not as good as you guys, so there maybe some misunderstanding in the post which you can tell me if there is anything I've done wrong. I hope you can understand :) All I want is just to solve the code issue.

@fregante
Copy link
Contributor

fregante commented Jul 4, 2015

@hwoarangzk I understand the issue, sorry if I was too harsh. Just so you know, "Really expect it coming soon" could have been "I hope it will be fixed soon," it sounds better :)

The code you posted in #385 is not valid, as you can test on http://validator.w3.org/nu/

@hugowetterberg
Copy link
Contributor

Ok, it seems like this issue is becoming a catch-all for robust error reporting vs. just hanging. If the issue is that even properly escaped quotes trigger an inf loop then you should reduce your example to just that and not include the other templating stuff. Looking at your example it could just as well be the later markup that causes the issue:

<i><%= status == 2 ? "pass" : (status == 3 ? "fail" : (status == 1 ? "undealt" : "")) %></i>

That is not valid html, and running your template language through a html tool will probably not work.

So: reduce, reduce, and reduce, until you get rid of all ambiguity. And if the problem actually is properly escaped quotes you can ask for the issue to be re-opened.

Even those who have English as their first language offend each other unintentionally (all the time). But just a unreserved "Sorry, I didn't mean it that way" can get things like that cleared away quick & nice.

@fregante
Copy link
Contributor

fregante commented Jul 4, 2015

It looks like a recurring problem (with templates) that #382 could fix :-/

@hwoarangzk
Copy link

@bfred-it That's ok :) Out of blows friendship grows
@hugowetterberg That's really my mistake not posted the actual html code with escaped double-quotes. I'll reopen that issue

@emirotin
Copy link

Encountered a similar issue - I had an equal sign missing between the attribute name and value:

<input type="number" max"{{ some angular interpolation }}">

@johngrogg
Copy link

Similarly, I accidentally had replaced a = with a - in an html attribute. I agree with @hugowetterberg that this all boils down to finding a way to catch infinite loops and report back at least some sort of generic error instead of just hanging.

Has anyone been able to track down where in the code the hanging is occurring? Is it lots of places depending on the mistake in the html code, or would there be one or two spots that are the culprits?

@GriffinSauce
Copy link

@hugowetterberg I'm not fully following you but are you suggesting that invalid html leading to 100% cpu hang is not an issue? Because in my opinion regardless of the input, be it expected, unexpected, malformed or invalid, a module should never hang.

@hugowetterberg
Copy link
Contributor

@GriffinSauce Nope, I'm definitely not suggesting that. We don't want software to hang. Detecting infinite loops / halting state is, however, a pretty hard problem https://en.wikipedia.org/wiki/Halting_problem so that wasn't really what I argued for either @johngrogg. And programmatically detecting unreasonable resource usage is out of scope for the project. The normal thing to do after a bug like this would be to create a regression test that makes sure that that it doesn't resurface.

After I was pulled into this thread (by a mention-mistake 😄) I just argued for the virtue of sticking to the topic of a thread, reducing any examples to the minimum required to reproduce a bug, and taking care not to sound like an entitled ass (because it's a very easy mistake to do).

But I'm actually not involved in this project, nor do I use it, I pretty much just responded because I wanted to help clearing up the communication issues.

@alexlamsl
Copy link
Collaborator

I think this is fixed by #510 - please reopen if that's not the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests