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
type="text/javascript" is redundant #214
Conversation
<script> elements only need an explicit type when it is something other than JavaScript. This is fully backwards compatible and compliant with HTML5.
👍 |
Pardon the sloppy work. Embarrassing. Thank you, Travis. |
Travis failed do to an unrelated server error. Thought I believe it should be fine now. |
@Aeyoun I restarted the build - let's see how it turns out. |
@halfdan, it failed again. I do not think it is related to these changes, though. https://travis-ci.org/piwik/piwik/builds/17798758 |
Please revert the change to piwik.js and js/piwik.js. Removing type="text/javascript" is ok for the UI because it's designed for html5. But the tracker aims for cross-browser compatibility, including older browser versions. |
TL;DR; if the @robocoder, this change will not create any compatibility issues. Every browser who ever supported JavaScript will support Including such marvelous legacy browsers as Firefox 2 and IE 5 (could not get a hold of even older IEs to test). This modernization really does not have any compatibility issues. It just makes the script more light-weight. Including it is an old habit, however, it is time for it to die. It is easy to test this. I checked this myself by running <script>window.alert(1)</script> through browsershots.org. Every single legacy and modern browser showed up with an alert dialog. http://www.w3.org/html/wg/drafts/html/master/scripting-1.html#the-script-element |
Any unanswered concerns? |
I'll try to find time next week to try it and will merge, as long as there is no other concern by anybody? |
Thanks for the PR! Feedback
Hopefully you can update the pR with these changes and then I'll merge it! |
If google analytics and google tag manager are anything to go by, they have removed text/javascript from the tags, even though it may be correct to do so, most some will still complain about that. Most major sites still use text/javascript at least in some parts (e.g. yahoo.com is quite inconsistent about that). I'm not sure if this can be easily done, but the best way to deprecate the old version would be to have checkbox that turns off the text/javascript attribute in the tag and in the contruction of the script element. (Assume that not everybody will agree on the topic supporting a choice is probably best) |
I’ll leave this open for discussion for a little longer before acting on @mattab‘s suggestion. If anyone want to do independent compatibility testing, that would be very welcome. |
Reading up on the topic, your change is not compliant with XHTML and html 4, where the type attribute is a mandatory element. While I am certainly with you when you want to modernize the web, changing this and breaking validation of pages that are not strictly html5, is not a good idea. |
I specifically checked and I can see the google analytics tag given in the "tracking code" section still has the type= attribute... However the new Universal analytics code does not have the type= attribute. I guess therefore that it's OK to not put it, but as I said I'm not keen to change this without testing more... Btw we have javascript tests at piwik/tests/javascript/ so these could maybe run in browsershots to check the tests still pass |
Sorry, I checked my analytics account, but apparently I upgraded all sites to universal already, the ga code likely uses type="text/javascript" w3c validation differs between html4 and 5: valid: http://validator.w3.org/check?uri=http%3A%2F%2Fwww.lehmann.cx%2Fpiwik-test%2Fhtml5.html |
Validation is a means not a goal. Compatibility should be the goal. |
What is our decision regarding this PR? is anyone against it? if not let's just do it... 👍 |
👍 |
@Aeyoun could you please update the pull request so I can merge it with latest git? thanks in advance. |
@mattab, I can do that in two weeks time unless someone wants to do it first. (I do not have time for it before then.) |
@Aeyoun sounds good! please re-base + re-open the pull request if/when you can. |
Just a small optimization/modernization to move the web forward.
The
<script>
elements only need an explicit type when it is somethingother than JavaScript.
This is fully backwards compatible and compliant with HTML5.