Skip to content

Infinite loop #321

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.

Sign up for GitHub

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

Jump to bottom
Closed
tspivey opened this issue Nov 25, 2015 · 8 comments
Closed

Infinite loop #321

tspivey opened this issue Nov 25, 2015 · 8 comments
Labels
Bug
Milestone
5.1

Comments

@tspivey
Copy link

tspivey commented Nov 25, 2015

Originally found when trying to tidy http://www.fimfiction.net/

Pared it down to this which seems to loop.

<!doctype html>
<html>
<body>
<ul>
<a><li></li></a>
<a href="www.example.org"><li></li></a>
</ul>
</body>
</html>
@benkasminbullock
Copy link
Contributor

Commits prior to 2388fb0 don't have this problem, so the issue was introduced in that commit.

@benkasminbullock
Copy link
Contributor

I think this fixes the problem:

https://github.com/benkasminbullock/tidy-html5/commit/71d963844897a6864097d4bc36501a97ff314e40

Can someone check that and pull if it is OK?

@geoffmcl
Copy link
Contributor

@tspivey thanks for reporting...

@benkasminbullock added your patch and can confirm this removes the infinite loop from this sample...

Can you do a PR? I tried the usual fetch from your fork and cherry pick the commit to keep you as the author but this failed! Or I can just push the patch?

@geoffmcl geoffmcl added the Bug label Nov 25, 2015
@geoffmcl geoffmcl added this to the 5.1 milestone Nov 25, 2015
@CMB
Copy link
Contributor

CMB commented Nov 25, 2015

@geoffmcl Something like the following should work to pull and apply
the patch. Current directory assumed to be the tidy-html5 source:

wget -O commit.patch https://github.com/benkasminbullock/tidy-html5/commit/71d963844897a6864097d4bc36501a97ff314e40.patch
git am commit.patch

And voila, patch applied, with proper credit.

I can also confirm that the patch from @benkasminbullock fixes this.

@geoffmcl geoffmcl mentioned this issue Nov 25, 2015
Merged
@benkasminbullock
Copy link
Contributor

You can also pull the patch into a local repository using

git pull [email protected]:benkasminbullock/tidy-html5.git

then push it to the github repository using

git push

or whatever command you usually use to update the github repository from your local copy.

geoffmcl added a commit that referenced this issue Nov 26, 2015
@geoffmcl
Issue #321 - bump v 5.1.27 for #322 PR - thanks
db4f647
@geoffmcl
Copy link
Contributor

@CMB thanks for that option... will keep that in mind as an alternative in future...

@benkasminbullock well I tried a similar approach -

$ git fetch [email protected]:benkasminbullock/tidy-html5.git
$ git log # to find the commit I wanted - this failed!
$ git cherry-pick commit

So as indicated this failed in this case! Not sure why? It usually works. $ git pull would auto merge all the fork commits, which in this case was only one, so would have worked fine...

But thanks for the PR #322, now merged... and version pushed to 5.1.27

@geoffmcl
Copy link
Contributor

@tspivey if you get a chance to pull 5.1.27, and if fixed, close this... thanks...

@geoffmcl
Copy link
Contributor

geoffmcl commented Dec 5, 2015

Will close this... feel free to reopen, or add a new issue...

@geoffmcl geoffmcl closed this as completed Dec 5, 2015
@geoffmcl geoffmcl mentioned this issue Apr 3, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
5.1
Development

No branches or pull requests
4 participants
@CMB @benkasminbullock @tspivey @geoffmcl