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

Player "officialImageSrc" should default to https #94

Open
danielmahon opened this issue Aug 27, 2018 · 4 comments
Open

Player "officialImageSrc" should default to https #94

danielmahon opened this issue Aug 27, 2018 · 4 comments

Comments

@danielmahon
Copy link

danielmahon commented Aug 27, 2018

Not a high priority of course but thought I would create the issue. The officialImageSrc property returned from the player object on the players endpoint (and any other endpoint it exists on) should probably default to https as the static.nfl.com API seems to support it.

Example

{
	lastUpdatedOn: "2018-07-23T13:52:46.361Z",
		{
			player: {
				id: 8281,
				firstName: "B.J.",
                                  // This should default to the https protocol
				officialImageSrc: "http://static.nfl.com/static/content/public/static/img/fantasy/transparent/200x200/DAN454194.png",
                                ...
			},
		},
		...
	]
}
@bradbarkhouse
Copy link
Collaborator

Is that really necessary tho? It's just a simple GET request for binary image data. Based on NFL.com's own player profile pages, they use regular HTTP.

@fredleblanc
Copy link

I don't have much of a horse in this particular race, but I've hit this issue before elsewhere: if your site loads over https, all of the resources on that page must come from https too, or else you're probably going to trigger a mixed-content warning.

Browsers seem to change how they handle this warning over time, but Google agrees it's a thing not to do.

(I agree that it's just an image, and shouldn't be a big deal, but in terms of ease-of-use: https on an http page — no potential mixed-content warning; http on an https page — probably a mixed-content warning.)

@bradbarkhouse
Copy link
Collaborator

bradbarkhouse commented Feb 1, 2019

In cases like these, would it be better to use the special "//" prefix when listing images (or other links)?

However, since the host supports it in this case, it's a pretty simple matter to switch to "https".

@fredleblanc
Copy link

I love that idea myself, because it should work as is, but if you really did need the http(s) in front of it, it's concatenation instead of find/replace.

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