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

Drop attribute "type" for style tags #4804

Merged
merged 5 commits into from Jan 3, 2017
Merged

Conversation

R-J
Copy link
Contributor

@R-J R-J commented Nov 16, 2016

Vanilla uses html5 and in html5 the default type is "text/css". Based on that information, this type is the only supported type anyway. So even if browsers wouldn't be compatible with html5, they would treat a simple <style> tag correctly.

Vanilla uses html5 and in html5 the default type is "text/css". Based on [that information](http://www.w3schools.com/tags/att_style_type.asp), this type is the only supported type anyway. So even if browsers wouldn't be compatible with html5, they would treat a simple `<style>` tag correctly.
@linc linc added this to the Sprint 26 milestone Nov 18, 2016
@linc linc added the Overdue label Dec 3, 2016
@linc linc modified the milestones: Sprint 27, Sprint 26 Dec 3, 2016
@@ -192,7 +192,7 @@
</style>
<!--[if (gte mso 9)|(IE)]>
[[literal]]
<style type="text/css">
<style>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some email clients are not yet HTML5 compliant, so I don't feel comfortable removing this yet.

@@ -6,7 +6,7 @@
<!--<![endif]--><meta name="viewport" content="width=device-width, initial-scale=1.0">
<!--[if (gte mso 9)|(IE)]>
{literal}
<style type="text/css">
<style>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some email clients are not yet HTML5 compliant, so I don't feel comfortable removing this yet.

@R-J
Copy link
Contributor Author

R-J commented Dec 7, 2016

Mail templates now have the type attribute again

@@ -6,7 +6,7 @@
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<!--<![endif]-->
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<style>
<style type="text/css">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one isn't necessary. This file gets compiled into a Smarty template (email/email-basic.tpl) where this block of css gets inlined and the block disappears. The email/email-basic.tpl file is the actual email template that gets used.

@beckyvb beckyvb added Revisit and removed Overdue labels Dec 16, 2016
@tburry tburry added the Overdue label Dec 20, 2016
@linc linc removed the Overdue label Dec 21, 2016
@linc linc modified the milestones: 2017-Q1-2, Sprint 27, 2017-Q1-1 Dec 26, 2016
@beckyvb beckyvb removed the Revisit label Jan 3, 2017
@beckyvb beckyvb merged commit be8c91e into vanilla:master Jan 3, 2017
@beckyvb
Copy link
Contributor

beckyvb commented Jan 3, 2017

Thank you :)

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

4 participants