Skip to content

bing https logo fix #3211

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

Merged
merged 2 commits into from
Feb 9, 2015
Merged

bing https logo fix #3211

merged 2 commits into from
Feb 9, 2015

Conversation

photostu
Copy link
Contributor

@photostu photostu commented Feb 5, 2015

If https is missing from bing logo uri, replace

If https is missing from bing logo uri, replace
@pagameba
Copy link
Member

pagameba commented Feb 5, 2015

why not use a protocol-less URI (//...)? No code required.

@bartvde
Copy link
Member

bartvde commented Feb 5, 2015

@pagameba the bing Maps metadata sends back an explicit http://

fix syntax to please Travis
@photostu
Copy link
Contributor Author

photostu commented Feb 5, 2015

I like the idea of removing the protocol as well, apparently this is considered an anti-pattern now though.
I updated my PR to fix syntax for Travis, my first time submitting here, please be gentle.

@pagameba
Copy link
Member

pagameba commented Feb 5, 2015

@bartvde ah :)

@tschaub
Copy link
Member

tschaub commented Feb 9, 2015

Thanks for the contribution @photostu. I think this looks like a sensible fix until Bing advertise the logo URL with the correct protocol.

tschaub added a commit that referenced this pull request Feb 9, 2015
@tschaub tschaub merged commit 776ffa9 into openlayers:master Feb 9, 2015
@photostu
Copy link
Contributor Author

photostu commented Feb 9, 2015

No problem, glad I could contribute! :-)

@bartvde
Copy link
Member

bartvde commented Feb 9, 2015

@photostu have you signed a CLA already? If not, please see: https://github.com/openlayers/cla for details. TIA.

@photostu
Copy link
Contributor Author

photostu commented Feb 9, 2015

I had not signed before, but just took care of it. Cheers.

@bartvde
Copy link
Member

bartvde commented Feb 9, 2015

thanks!

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.

4 participants