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

Make placeholder images https enabled. #1951

Closed

Conversation

tochev
Copy link

@tochev tochev commented Sep 27, 2014

Loading http resources on https connection causes warnings and browsers,
depending on their configuration, will refuse to load them.

The change from placehold.it to dummyimage.com was necessitated by the
lack of https support on the later.

Loading http resources on https connection causes warnings and browsers,
depending on their configuration, will refuse to load them.

The change from placehold.it to dummyimage.com was necessitated by the
lack of https support on the later.
@amercader amercader self-assigned this Sep 30, 2014
@amercader
Copy link
Member

@tochev Thanks for this. Rather than relying on a third party service, I think a much better solution than the current one is serving the placeholder image from CKAN itself, to avoid other problems with network connectivity.
Can you add these two images (and any other that is using placehold.it) to the images folder and change the templates to something like src={{ h.url_for_static('/base/images/placeholder.png') }}

https://github.com/ckan/ckan/tree/master/ckan/public/base/images

For extra bonus, if you add locally the "open data" badge on the footer as well that would be awesome :)

https://github.com/ckan/ckan/blob/master/ckan/templates/footer.html#L17

Thanks

@tochev
Copy link
Author

tochev commented Sep 30, 2014

I will do it this weekend.

@amercader amercader modified the milestones: CKAN 2.4, CKAN 2.3 Oct 22, 2014
@amercader
Copy link
Member

@tochev do you want to have a go a this? otherwise I'll do it

@amercader
Copy link
Member

Closed in favour of #2119, thanks @tochev for the original patch

@amercader amercader closed this Dec 5, 2014
@tochev
Copy link
Author

tochev commented Dec 5, 2014

@amercader , sorry, I got caught in other things.

amercader added a commit that referenced this pull request Dec 5, 2014
@amercader amercader removed their assignment Dec 9, 2014
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

Successfully merging this pull request may close these issues.

None yet

2 participants