Implements the addJquery setting option that provides better control over DebugKit inserting jquery

[ndejong]

No description provided.

[dereuromark]

as per cakephp coding standards there should be a space after the "if"

[jippi]

@markstory any thoughts on this PR other than the phpcs ?

[markstory]

I don't understand why it is needed. I was pretty sure adding jQuery in noConflict mode was safe to the host page outside of having to download jQuery twice that is.

[josegonzalez]

People don't like placing jquery on the page twice I guess.

[markstory]

I'm not a fan of having debugkit randomly break because the version of jquery is incompatible though.

[jippi]

we had huge problems getting it to work nicely at bownty due to double inclusion of jquery, so i did a hack similar to this one to make it behave, so having an official and non-hacky way of doing it would surely help us going forward

if you use this feature, it would be "on your own risk" imo though - for most apps it won't be a problem

[markstory]

Perhaps I misunderstand how no conflict is supposed to work, but I thought one of its main purposes was to support multiple jquery libraries on the same page.

[jippi]

afaik noConflict() just unbinds jQuery from the $ shorthand :) - it keeps the jQuery object unless it's used as in example "Completely move jQuery to a new namespace in another object"

[ADmad]

@markstory No :) It's to prevent the $ usage conflict with other js libraries.

[ADmad]

Can't we do something like:

if (!window.jQuery) {
    //load jquery 
}

[ravage84]

I would prefer a solution like @ADmad 's

[dereuromark]

You can always do the following

var myJ = jQuery.noConflict(true);

Then you can use myJ anywhere (and should not use $ or jQuery namespace anymore) and it will not conflict with any other jQuery library included.

[jippi]

@dereuromark sure, but that will use the first found jQuery object in the dom, the way it's injected now, makes it hard to know if our jQuery or debug kit's jQuery gets loaded into myJ - the safest and most simple way is to just configure a 'don't include debugkit jquery kthxbye' switch

[dereuromark]

@jippi I agree with you there.

[AD7six]

The problem with the existing js is that you need to include https://github.com/cakephp/debug_kit/blob/master/webroot/js/js_debug_toolbar.js for the toolbar to work (duh). However this line effectively assumes that there are two jquery files loaded. Including js_debug_toolbar.js will do two things:

• Set DEBUGKIT.$ to the current value of $
• restore $ to whatever it was previously

So, if these js files are included in this order:

• app/jquery.js
• debug_kit/jquery.js
• debug_kit/js_debug_toolbar.js

When js_debug_toolbar runs this happens:

• $ is already defined as debug_kit/jquery
• DEBUGKIT.$ is defined correctly
• $ is restored to app/jquery
• everything works

however, if debug_kit/jquery is not loaded (which is logical, there's no need to include jquery twice) this happens:

• $ is already defined as app/jquery
• DEBUGKIT.$ is defined correctly (assuming no version problem with app/jquery)
• $ is restored to undefined
• debug kit works, nothing else does

This is the problem whereby app code alternately breaks/works when including/removing debug kit's toolbar js.

[markstory]

I still think there is a reason to include jQuery twice. For example if the host page is using jQuery 1.9+ parts of DebugKit don't work (from what I remember). Alternatively if DebugKit is updated to use 1.9+ and then the host page uses an older version the toolbar will also break. I'm hesitant to let the page provide jQuery as not all of them will, and the ones that do can't make any promises about including the correct version of jquery.

Would including the toolbar js + debugkit jquery at the bottom of the page help the issue? Another option is to modify debugkit's jquery file so it never exports to window. I'd like to avoid that as it makes upgrading a pain.

[jippi]

I think the setting should remain "on" by default - if people want to mess with jQuery and DebugKit on their own, let them fight that fight, we use the latest stable at bownty, and everything just works out of the box :) don't know about other jQuery versions though, but they probably worked around the double jquery problem already in their apps :)

Think the PR simply makes it easier to apply a hack tons of people probably have in place already

[AD7six]

maybe a solution like this is appropriate, only include the toolbar js directly and modify as such:

if (window.jQuery && jQuery.fn.jquery === '1.8.1') { // example - should be more forgiving
    DEBUGKIT.$ = jQuery;
} else {
    document.write('<script src="/debug_kit/js/jquery.js"><\/script>'); // syncronous load
    DEBUGKIT.$ = jQuery.noConflict(); // do not unset window.jQuery
}

moving things around doesn't help if the toolbar js assumes that there will always be multiple jquery files loaded, there will always be a permutation whereby it leaves the user with no variable pointing at jQuery if the deleteAll parameter is used with noConflict.

[markstory]

I would prefer something like @AD7six more settings = more problems. The code should attempt to do the best thing where possible. Something like Andy's change would fix the issues with version craziness and simultaneously remove the multiple jquery problem.

[markstory]

Closing in favor of the approach @AD7six suggested, which I made into pull request #97