-
Notifications
You must be signed in to change notification settings - Fork 67
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
Revert #301 Add SRI #308
Revert #301 Add SRI #308
Conversation
|
||
<!--[if IE 8]><link href="<%= asset_path "fonts-ie8.css" %>" media="all" rel="stylesheet" /><![endif]--> | ||
<!--[if gte IE 9]><!--><%= stylesheet_link_tag "fonts.css", media: "all", integrity: true, crossorigin: "anonymous" %><!--<![endif]--> | ||
<!--[if gte IE 9]><!--><%= stylesheet_link_tag "fonts.css", media: "all", integrity: true, crossorigin: "anonymous" %><!--<![endif]--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this was duplicated accidentally in original?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think so.
<!--[if gte IE 9]><!--><%= stylesheet_link_tag "fonts.css", media: "all", integrity: true, crossorigin: "anonymous" %><!--<![endif]--> | ||
<!--[if lt IE 9]><%= javascript_include_tag "ie.js", integrity: true, crossorigin: "anonymous" %><![endif]--> | ||
<!--[if gte IE 9]><!--><link href="<%= asset_path "fonts.css" %>" media="all" rel="stylesheet" /><!--<![endif]--> | ||
<!--[if lt IE 9]><script src="<% asset_path "ie.js" %>"></script><![endif]--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asset_path is missing an equals here, should be: <%=
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spotted via this comparison: 10d2348...revert-add-sri
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops. I blame having to make this change 4 times because I kept editing the built version instead of the source! Thanks for spotting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in a rebase.
This is a manual revert, as the PR could not be reverted automatically. We're reverting this PR because there is a bug in the SRI implementation of Firefox versions upto 52 which at time of writing accounts for 0.7% of total traffic (~315k users). We still want to implement SRI, but for now we're holding off until we'd impact fewer users.
This is a manual revert of #301, as the PR could not be reverted automatically.
We're reverting this PR because there is a bug in the SRI implementation
of Firefox versions upto 52 which at time of writing accounts for 0.7% of
total traffic (~315k users). We still want to implement SRI, but for now
we're holding off until we'd impact fewer users.