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

HTML: don't remove http(s) from links #40

Closed
stroborobo opened this issue Aug 20, 2015 · 5 comments
Closed

HTML: don't remove http(s) from links #40

stroborobo opened this issue Aug 20, 2015 · 5 comments

Comments

@stroborobo
Copy link

I'm minifing HTML for a site that's available via HTTPS, but there's a link pointing to a site not supporting HTTPS. Minify removes "http:" from links like this:

> echo '<a href="http://kbct.de/">foo</a>' | minify -x .html
<a href=//kbct.de/>foo</a>

Now that link adopts the protocol from the page it's on, in my case HTTPS, which breaks the link.

@stroborobo stroborobo changed the title HTML: don't remove the protocol HTML: don't remove http(s) from links Aug 20, 2015
@stroborobo
Copy link
Author

Ah, I see, you want people to use the new rel="external" attribute on anchor elements. IMHO all links containing a domain are to be considered external, what do you think?

@tillberg
Copy link

I too had this issue. I serve my sites only over https, and I already specify protocol for links only when sites do not support https, so for my own use I just removed the block of code that deals with this: tillberg@cfaa3ee (I'd be happy to turn that into a PR if desired, but it doesn't seem friendly to just go and create a PR for something that basically just nukes someone else's code :) )

I agree with @Knorkebrot above. I get that there might be some use case for trying to strip the protocol off of pages that include them in self-references (e.g. some heavy server-side application frameworks do this, I think), but that seems like a small benefit to get compared to requiring everyone to mark up their anchors with rel="external" whenever they need to force the link protocol. (I also had never heard of rel="external" before, so I might be biased by my own ignorance.)

@tdewolff
Copy link
Owner

Yes you are right, it's not ideal. I will make changes to keep the protocol. It's a pity because stripping http: can considerably reduce the size.

@tdewolff
Copy link
Owner

tdewolff commented Sep 6, 2015

Fixed in 3b07a59 on the develop branch. It will be re-enabled in the future when passing options is possible, including the currently used protocol. Either http or https can then be omitted, but not both.

@tdewolff tdewolff closed this as completed Sep 6, 2015
@tdewolff
Copy link
Owner

Please check the options branch, it contains an implementation of passing options to the minifier. HTML now has the option scheme which can be either http or https and it will remove only that protocol for links. Call m.Set("scheme", "http") to enable that.

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

No branches or pull requests

3 participants