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

protocol relative src broken #114

Open
IntranetFactory opened this issue May 21, 2017 · 4 comments
Open

protocol relative src broken #114

IntranetFactory opened this issue May 21, 2017 · 4 comments

Comments

@IntranetFactory
Copy link

It seems that e7ef3ae#diff-3005a2eaefe34d5d0afb8d08687f8aaeR394 has broken protocol relative url in src like

"//img.youtube.com/vi/18OXnzrw1V1/maxresdefault.jpg" is e.g. resolved to "http://localhost//img.youtube.com/vi/18OXnzrw1V1/maxresdefault.jpg"

expected: the value of a protocol relative src url should not be changed

the following change fixed the issue for me:

if (resolved[0] === '/') to if (resolved[0] === '/' && resolved.indexOf('//') != 0)

@stramel
Copy link
Contributor

stramel commented May 26, 2017

This is considered an anti-pattern. Not sure they will want to implement it.

https://www.paulirish.com/2010/the-protocol-relative-url/

@IntranetFactory
Copy link
Author

It might be seen as an anti-pattern for public sites. But there are a lot intranet sites not using SSL. We have elements which can be used both on public facing and internal sites, and these elements will show “This Page Contains Both Secure and Non-Secure Items” without protocol-relative URLs.

Besides that did protocol-relative URLs worked in the past and had been broken without notice.

@govis
Copy link

govis commented Sep 18, 2017

broken still? https is not always the case, I think that there should be a way to work with and without SSL (without changing the code).

@AlbertoFdzM
Copy link

IMO this issue should be fixed, if not, use the semantic versioning to remove this feature with a minor/major update providing support to previous versions to not broke dependants component usage.

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

4 participants