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

Fix 404 page widget script URL #1324

Closed
wants to merge 1 commit into from
Closed

Fix 404 page widget script URL #1324

wants to merge 1 commit into from

Conversation

alfredxing
Copy link

Fix 404 page widget script URL to be compatible with HTTPS by replacing http:// with //.

I checked the src and Google does have a hosted copy on SSL (https://linkhelp.clients.google.com/tbproxy/lh/wm/fixurl.js).

Fix 404 widget script URL to be compatible with HTTPS by replacing
`http://` with `//`. The script hosted by Google is available on both
protocols.
@necolas
Copy link
Member

necolas commented Mar 4, 2013

Looks good to me. I can't remember if there is a reason for this not to be protocol relative, but if there is, I couldn't find it.

Also, thanks for the great commit message in line with the contribution guidelines. Having clear explanations in the commit messages makes all the difference when you have to do some archaeology on a codebase. Much appreciated.

@necolas
Copy link
Member

necolas commented Mar 4, 2013

Merged in a104609

@necolas necolas closed this Mar 4, 2013
@alfredxing
Copy link
Author

I don't the code Google provided was not protocol relative...

Thanks!

@alfredxing alfredxing deleted the 404-widget-url branch March 4, 2013 17:05
@alfredxing alfredxing restored the 404-widget-url branch March 4, 2013 17:05
@mathiasbynens
Copy link
Member

Did anyone verify this code doesn’t suffer from the same problem as the Google Analytics snippet? See #1319. (As long as we still support IE6…)

@necolas
Copy link
Member

necolas commented Mar 4, 2013

No.

@mathiasbynens
Copy link
Member

I still haven’t tested this in IE6, but it seems to be alright:

$ echo -e "GET / HTTP/1.1\nEOT" | openssl s_client -connect linkhelp.clients.google.com:443 2>&1 | grep subject
subject=/C=US/ST=California/L=Mountain View/O=Google Inc/CN=*.google.com

linkhelp.clients.google.com matches *.google.com, so this change shouldn’t cause any errors, not even in IE6. (Bash snippet from http://mathiasbynens.be/notes/async-analytics-snippet#comment-29.)

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.

3 participants