Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Simplify inline script loader #86

Merged
merged 1 commit into from

2 participants

@josh

At GitHub, we're trying to roll out CSP, which means no inline scripts. I'd really to have a "noInlineScript" option for mini-profiler. This patch moves in that direction by simplifying the inline script loader and moving the logic into the include.js itself.

Ideally, I'd like to see the injected script just be something like:

<script type="text/javascript" async defer
  src="/mini-profiler-resources/includes.js"
  data-version="{version}"
  data-current-id="{currentId}"
  data-render-position="{position}"
  data-show-trivial="{showTrivial}"
  data-show-children-time="{showChildren}"
  data-authorized="{authorized}">

The jQuery loader and deferInit function are moved into include.js's init function.

Second, leaking jQueryMP kinda bothers me. Since we are loading it after the MiniProfiler object is set, we can keep a scoped reference at MiniProfiler.jQuery.

Related question, why is useExistingjQuery even an option? Really, why isn't it the default? For Rails at least, it really should be.

/cc @SamSaffron @rtomayko

@josh josh Simplify inline script loader
Loads jQuery on MiniProfiler.init
Moves jQueryMP global to MiniProfiler.jQuery
8375517
@SamSaffron SamSaffron merged commit 5d89eeb into SamSaffron:master
@SamSaffron
Owner

I very much support your suggestion, keeping the injected code minimal is fine!

re useExistingjQuery I tend to agree we could strip this setting provided we test for compatible versions of jQuery. Clearly MP will not work if you have jQuery 1.0 in your page. We would need to test which versions of jQuery work with mp and then only load its own version of jQuery if an earlier rev is there.

@josh

Awesome, thanks Sam.

Will probably work on that inline script change soon. Do you think we want to support both code paths, the existing inline script and a data-* encoded style?

For the useExistingjQuery option, I'll go through and test them all!

@SamSaffron
Owner

also one concern around useExistingjQuery is people running jQuery edge, something to keep in mind.

I don't think we need both styles of loading the profiler, happy to simply move to the data-* style provided browser support is good, I would like MP to work on IE8+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 7, 2012
  1. @josh

    Simplify inline script loader

    josh authored
    Loads jQuery on MiniProfiler.init
    Moves jQueryMP global to MiniProfiler.jQuery
This page is out of date. Refresh to see the latest.
View
81 StackExchange.Profiling/UI/include.partial.html
@@ -1,62 +1,33 @@
<script type="text/javascript">
(function(){{
- var init = function() {{
- var load = function(s,f){{
- var sc = document.createElement('script');
- sc.async = 'async';
- sc.type = 'text/javascript';
- sc.src = s;
- var l = false;
- sc.onload = sc.onreadystatechange = function(_, abort) {{
- if (!l && (!sc.readyState || /loaded|complete/.test(sc.readyState))) {{
- if (!abort){{l=true; f();}}
- }}
- }};
-
- document.getElementsByTagName('head')[0].appendChild(sc);
- }};
-
- var initMp = function(){{
- load('{path}includes.js?v={version}',function(){{
- MiniProfiler.init({{
- ids: {ids},
- path: '{path}',
- version: '{version}',
- renderPosition: '{position}',
- showTrivial: {showTrivial},
- showChildrenTime: {showChildren},
- maxTracesToShow: {maxTracesToShow},
- showControls: {showControls},
- currentId: '{currentId}',
- authorized: {authorized}
- }});
- }});
- }};
- if ({useExistingjQuery} && typeof(jQuery) === 'function') {{
- jQueryMP = jQuery;
- initMp();
- }} else {{
- load('{path}jquery.1.7.1.js?v={version}', initMp);
+ var load = function(s,f){{
+ var sc = document.createElement('script');
+ sc.async = 'async';
+ sc.type = 'text/javascript';
+ sc.src = s;
+ var l = false;
+ sc.onload = sc.onreadystatechange = function(_, abort) {{
+ if (!l && (!sc.readyState || /loaded|complete/.test(sc.readyState))) {{
+ if (!abort){{l=true; f();}}
}}
-
+ }};
+ document.getElementsByTagName('head')[0].appendChild(sc);
}};
- var w = 0;
- var f = false;
- var deferInit = function(){{
- if (f) return;
- if (window.performance && window.performance.timing && window.performance.timing.loadEventEnd == 0 && w < 10000){{
- setTimeout(deferInit, 100);
- w += 100;
- }} else {{
- f = true;
- init();
- }}
- }};
- if (document.addEventListener) {{
- document.addEventListener('DOMContentLoaded',deferInit);
- }}
- var o = window.onload;
- window.onload = function(){{if(o)o(); deferInit()}};
+ load('{path}includes.js?v={version}',function(){{
+ MiniProfiler.init({{
+ usingExistingjQuery: {useExistingjQuery},
+ ids: {ids},
+ path: '{path}',
+ version: '{version}',
+ renderPosition: '{position}',
+ showTrivial: {showTrivial},
+ showChildrenTime: {showChildren},
+ maxTracesToShow: {maxTracesToShow},
+ showControls: {showControls},
+ currentId: '{currentId}',
+ authorized: {authorized}
+ }});
+ }});
}})();
</script>
View
50 StackExchange.Profiling/UI/includes.js
@@ -1,5 +1,6 @@
"use strict";
-var MiniProfiler = (function ($) {
+var MiniProfiler = (function () {
+ var $;
var options,
container,
@@ -547,24 +548,47 @@ var MiniProfiler = (function ($) {
document.getElementsByTagName('head')[0].appendChild(sc);
};
- if (options.authorized) {
- var url = options.path + "includes.css?v=" + options.version;
- if (document.createStyleSheet) {
- document.createStyleSheet(url);
+ var wait = 0;
+ var finish = false;
+ var deferInit = function() {
+ if (finish) return;
+ if (window.performance && window.performance.timing && window.performance.timing.loadEventEnd == 0 && wait < 10000) {
+ setTimeout(deferInit, 100);
+ wait += 100;
} else {
- $('head').append($('<link rel="stylesheet" type="text/css" href="' + url + '" />'));
+ finish = true;
+ init();
}
+ };
- if (!$.tmpl) {
- load(options.path + 'jquery.tmpl.js?v=' + options.version, doInit);
- } else {
+ var init = function() {
+ if (options.authorized) {
+ var url = options.path + "includes.css?v=" + options.version;
+ if (document.createStyleSheet) {
+ document.createStyleSheet(url);
+ } else {
+ $('head').append($('<link rel="stylesheet" type="text/css" href="' + url + '" />'));
+ }
+ if (!$.tmpl) {
+ load(options.path + 'jquery.tmpl.js?v=' + options.version, doInit);
+ } else {
+ doInit();
+ }
+ }
+ else {
doInit();
}
}
- else {
- doInit();
- }
+ if (options.useExistingjQuery && typeof(jQuery) === 'function') {
+ MiniProfiler.jQuery = $ = jQuery;
+ $(deferInit);
+ } else {
+ load(options.path + "jquery.1.7.1.js?v=" + options.version, function() {
+ MiniProfiler.jQuery = $ = jQuery.noConflict(true);
+ $(deferInit);
+ });
+ }
},
getClientTimingByName: function (clientTiming, name) {
@@ -784,7 +808,7 @@ var MiniProfiler = (function ($) {
return (duration || 0).toFixed(1);
}
};
-})(jQueryMP);
+})();
// prettify.js
// http://code.google.com/p/google-code-prettify/
View
2  StackExchange.Profiling/UI/jquery.1.7.1.js
1 addition, 1 deletion not shown
View
2  StackExchange.Profiling/UI/jquery.tmpl.js
@@ -483,4 +483,4 @@
jQuery.tmpl(null, null, null, this).insertBefore(coll[0]);
jQuery(coll).remove();
}
-})(jQueryMP);
+})(MiniProfiler.jQuery);
View
13 StackExchange.Profiling/UI/list.js
@@ -2,16 +2,17 @@
MiniProfiler.list = {
init:
function (options) {
+ var $ = MiniProfiler.jQuery;
var opt = options || {};
var updateGrid = function (id) {
- jQueryMP.ajax({
+ $.ajax({
url: options.path + 'results-list',
data: { "last-id": id },
dataType: 'json',
type: 'GET',
success: function (data) {
- jQueryMP('table tbody').append(jQueryMP("#rowTemplate").tmpl(data));
+ $('table tbody').append($("#rowTemplate").tmpl(data));
var oldId = id;
var oldData = data;
setTimeout(function () {
@@ -26,12 +27,12 @@ MiniProfiler.list = {
}
MiniProfiler.path = options.path;
- jQueryMP.get(options.path + 'list.tmpl?v=' + options.version, function (data) {
+ $.get(options.path + 'list.tmpl?v=' + options.version, function (data) {
if (data) {
- jQueryMP('body').append(data);
- jQueryMP('body').append(jQueryMP('#tableTemplate').tmpl());
+ $('body').append(data);
+ $('body').append($('#tableTemplate').tmpl());
updateGrid();
}
});
}
-};
+};
Something went wrong with that request. Please try again.