Skip to content

tidy incorrectly warns when <script> tags have html inside #281

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

Closed
joeheyming opened this issue Oct 2, 2015 · 16 comments
Closed

tidy incorrectly warns when <script> tags have html inside #281

joeheyming opened this issue Oct 2, 2015 · 16 comments
Labels
Milestone

Comments

@joeheyming
Copy link

Some javascript libraries use script tags for templates:
<script type="text/html" id="my-template">
<h1>hello world</h1>
</script>

When I put html with a script tag like this , tidy5 warns for me around any closing tag that is not a closing </script> tag:
line 383 column 97 - Warning: '<' + '/' + letter not allowed here

@geoffmcl
Copy link
Contributor

geoffmcl commented Oct 3, 2015

@joeheyming yes, that certainly seems an erroneous warning output...

Will look at it soonest, unless someone else beats me to it...

Thanks for reporting...

@geoffmcl
Copy link
Contributor

geoffmcl commented Oct 7, 2015

@joeheyming hmmmmm, that message usually warns that tidy has inserted an escape character into javascript...

But this conversion of / to \/ only happens if the container has no attributes, like just <script>, or has a type or lang attribute declaring it as "javascript"...

In this sample you have used type="text/html", so you see the warning but not the effect...

I guess this warning should be after the attribute check, and only issue it if tidy adds the escape character...

Since I am presently working in an experimental branch issue-65, have effected this change there for testing...

To get this version, please do

$ cd tidy-html5
$ git checkout issue-65
$ git pull
$ cd build/cmake
$ rm CMakeCache.txt [optional]
$ ./build-me.sh
$ ./tidy -v
$ HTML Tidy for <OS> version 5.1.14.EXP1

Hope you get a chance to re-test using verions 5.1.14.EXP1 and report... thanks...

@joeheyming
Copy link
Author

I just tested this out, thanks so much for looking at this.

I still am having the issue with my freshly built issue-65 branch. I ran your instructions and am pointing to my local ./tidy5 under build/cmake:

line 383 column 97 - Warning: '<' + '/' + letter not allowed here
line 384 column 37 - Warning: '<' + '/' + letter not allowed here

Here is the offending html:
<script type="text/html" id="some-template">
<i class="tiny some-icon" data-bind="css: 'some-css-class' + value.toLowerCase()"></i>
<span data-bind="text: label"></span>
</script>

It doesn't like the closing </i> tag and the closing </span> tag.

@geoffmcl
Copy link
Contributor

geoffmcl commented Oct 7, 2015

@joeheyming, unfortunately can not duplicate a problem with the html sample you give...

Is that a typo my local ./tidy5?

Tidy does NOT have a 5 appended since release 5.0.0 in June this year!

What do you get when you run ./tidy -v?

If you do not see 5.1.14.EXP1, especially the EXP1, then something is wrong in your checkout of issue-65 branch?

What branch displays when you run $ git status?

Very puzzled...

@joeheyming
Copy link
Author

ahah, that was it. Thanks, I've been using tidy5 since I wrote my html lint test back in June! :-P

Looks like it works!

@geoffmcl
Copy link
Contributor

@joeheyming have fully merged issue-65 branch to master and will get around to deleting it...

If you get a chance to pull, build, install master version 5.1.17 plus, and confirm this remains fixed, maybe we can close this issue? Thanks...

@balthisar
Copy link
Member

@joeheyming indicates the issue is resolved, and testing master on my system indicates that too, so I'll close the issue.

@joeheyming
Copy link
Author

If I install tidy with homebrew, will I be getting tidy with this issue resolved?
This is my version:

$$ tidy -v
HTML Tidy for Mac OS X version 5.0.0

@geoffmcl
Copy link
Contributor

@joeheyming no, I think not... my reading of http://braumeister.org/formula/tidy-html5 says that only has 5.0.0! This fix is only in 5.1.17++

You could add a comment to this commit, asking @zmwangx, @MikeMcQuaid to add the 5.1.25 release... it is a dropin replacement of 5.0.0... just change the url to https://github.com/htacg/tidy-html5/archive/5.1.25.tar.gz...

But why use homebrew? I note they have a backlog of some 250 PR, and a similar number of issues, so it looks like the homebrew maintainers have some catching up to do...

Why not continue to build your tidy from source, like it seemed you were doing at the beginning, then you can use the latest, 5.1.32, or even participate in testing of developmental branches...

Tidy needs your help!

@zmwangx
Copy link

zmwangx commented Dec 10, 2015

Happy to oblige, submitted Homebrew/legacy-homebrew#46874.

But why use homebrew?

It's awesome. You'll know if you've used it.

I note they have a backlog of some 250 PR, and a similar number of issues, so it looks like the homebrew maintainers have some catching up to do...

You'll know they're doing a good job if you also look at the number of issues/PRs closed... PR: 242 open, 30253 closed. Issues: 254 open, 16084 closed.

@MikeMcQuaid
Copy link

But why use homebrew? I note they have a backlog of some 250 PR, and a similar number of issues, so it looks like the homebrew maintainers have some catching up to do...

I think it's a bit rude to ping me by name and then say that, @geoffmcl. The reason we have a large backlog is because e.g. [we had 26 pull requests opened yesterday](repo:homebrew/homebrew created:2015-12-09 is:pr).

@joeheyming
Copy link
Author

Ok Ok. Thanks @ALL for all your help. I really appreciate it. @zmwangx Those stats are awesome, I wish I was able to make that big of an impact with open source. @geoffmcl Be careful what you wish for, maybe you'll get a pull request from me someday :-)

@geoffmcl
Copy link
Contributor

@joeheyming yes, the stats are awesome... sorry for the casual observation! And look forward to your PR so you can make an impact on open source ;=))

@joeheyming
Copy link
Author

@geoffmcl The reason I hesitate to build directly from the source is that I work with a team of other developers and they understand homebrew. The barrier to entry increases as you require manual steps. Basically I'm using tidy to block git pushes to my team's repository if any html is malformed.

@balthisar
Copy link
Member

@MikeMcQuaid, clearly @geoffmcl meant your stats are awesome. Is there anything I can do to help ensure we keep up to date with homebrew? I'm not the package manager for homebrew's tidy package, but as one of the Tidy team and a Mac user myself, I'll do what I can when I can.

@MikeMcQuaid
Copy link

@MikeMcQuaid, clearly @geoffmcl meant your stats are awesome. Is there anything I can do to help ensure we keep up to date with homebrew? I'm not the package manager for homebrew's tidy package, but as one of the Tidy team and a Mac user myself, I'll do what I can when I can.

@balthisar We sorted it out but thanks for clarifying. You could just submit new releases as formulae updates to Homebrew (if your buildsystem is the same it's just a URL and checksum change). Thanks!

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

5 participants