Edgewall Software
Modify

Opened 13 years ago

Closed 12 years ago

Last modified 11 years ago

#5954 closed defect (fixed)

Can't use prototype in plugins as $ conflicts with jquery

Reported by: rohitbrai@… Owned by: Christopher Lenz
Priority: normal Milestone: 0.11
Component: general Version:
Severity: normal Keywords: search.js jquery prototype
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

When you try to use prototype.js library in your plugins, The '$' function conflicts with the '$' in jquery.

This shows error in search.js that is available on all the plugin pages and it uses the $ from jquery, but since prototype is loaded later it overrides the $ and this causes an error in search.js when the plugin is opened.

The resolution is to use jquery in no conflict mode. Attached is the diff that implements this, and has to be applied to trac/htdocs/js/search.js

Attachments (5)

jsquery_noconflict_search.js.diff (2.2 KB ) - added by rohitbrai@… 13 years ago.
Patch to add noconflict mode to jquery in search.js
jquery_noconflict.diff (9.6 KB ) - added by Christopher Lenz 13 years ago.
Make the Trac Javascript files not assume that the jQuery function is bound to the global $ name
jquery_noconflict-r6245.diff (24.3 KB ) - added by Christian Boos 13 years ago.
More complete version of the patch, with "trac_" scoped global functions.
jquery-namespace.diff (22.3 KB ) - added by Paul Irish <paul.irishEWWSPAM@…> 12 years ago.
namespacing jQuery to $j, thus freeing up $ for Prototype
jquery-safeify.diff (68.2 KB ) - added by Paul Irish <paul.irishEWWSPAM@…> 12 years ago.
new jquery/prototype compatibility script

Download all attachments as: .zip

Change History (42)

by rohitbrai@…, 13 years ago

Patch to add noconflict mode to jquery in search.js

comment:1 by Christopher Lenz, 13 years ago

Can't you do this on the Prototype side?

comment:2 by rohitbrai@…, 13 years ago

I have searched quite a bit but couldn't find any way to do so. Also having it built in the search.js file made more sense as it becomes more plugin friendly and not everyone who writes a plugin and uses prototype would have to write any wrieing up code if there is any to make prototype play well with jquery.

Kind Regards, Rohit

comment:3 by Alec Thomas, 13 years ago

To be complete and consistent we'd really have to do this across all templates and .js files.

comment:4 by Noah Kantrowitz, 13 years ago

We have opted to use jQuery, I see no reason to make the code harder to read for the benefit of other JS toolkits. Sounds like over-generalizing to me. -1.

comment:5 by rohitbrai@…, 13 years ago

I don't say that you have to be compatible with all the JavaScript toolkits. JQuery is the choen one, soit can remain, but JQuery is good on DOM and UI Effects thing, One thing you really miss not using Prototype is the Classes and all support prototype provides. Another thing is Prototype has a large user and developer base and there is also a very large prototype codebase. If we don't allow users to use it you are literally locking out a large talent. Open is about choice and freedom and we should not force or bind people to our decisions.

in reply to:  5 ; comment:6 by Noah Kantrowitz, 13 years ago

Replying to rohitbrai@gmail.com:

I don't say that you have to be compatible with all the JavaScript toolkits. JQuery is the choen one, soit can remain, but JQuery is good on DOM and UI Effects thing, One thing you really miss not using Prototype is the Classes and all support prototype provides. Another thing is Prototype has a large user and developer base and there is also a very large prototype codebase. If we don't allow users to use it you are literally locking out a large talent. Open is about choice and freedom and we should not force or bind people to our decisions.

Sure, and you are free to take Trac and change it to use Prototype (or anything else for that matter). This is a design question, not one of ideologies. The Trac team uses jQuery, using jQuery in noConflict mode would make it that much more difficult for us. If Prototype can't be used without taking over the $ variable, I see this as an upstream problem.

in reply to:  6 ; comment:7 by anonymous, 13 years ago

Replying to nkantrowitz:

Replying to rohitbrai@gmail.com:

I don't say that you have to be compatible with all the JavaScript toolkits. JQuery is the choen one, soit can remain, but JQuery is good on DOM and UI Effects thing, One thing you really miss not using Prototype is the Classes and all support prototype provides. Another thing is Prototype has a large user and developer base and there is also a very large prototype codebase. If we don't allow users to use it you are literally locking out a large talent. Open is about choice and freedom and we should not force or bind people to our decisions.

Sure, and you are free to take Trac and change it to use Prototype (or anything else for that matter). This is a design question, not one of ideologies. The Trac team uses jQuery, using jQuery in noConflict mode would make it that much more difficult for us. If Prototype can't be used without taking over the $ variable, I see this as an upstream problem.

Sure it is a design question and question of a design that plays well with people having the best intentions for the product. Trac is widely adopted because it is the best tool I have seen and moreover it is very customizable and extensible. I won't agree that it is an upstream issue as JQuery provides us a way to play well with other libraries, so I think we can also play well with others.
I never mentioned moving trac to prototype. When Trac team uses it, it should be continued to be used. But when we can allow the developers outside to build plugins and directly deploy them to trac without modification to the base I feel it's a big deal.
And the effort is not a very large one. I have developed a whole dashboard plugin, which we plan to realease soon to open source. The only file hat gave a conflict was search.js, that I have patched to work in no conflict mode.I don't want to instruct my end users to download trac, apply a patch and install it and then you can use my plugin and more so difficult if you already have trac installed.
And I really don't understand how it can be difficult for you to use JQuery in no conflict mode. Typing in $J instead of $ should not be that difficult when it benefits your friends and supporters out there.

in reply to:  7 ; comment:8 by anonymous, 13 years ago

Replying to anonymous:

And I really don't understand how it can be difficult for you to use JQuery in no conflict mode. Typing in $J instead of $ should not be that difficult when it benefits your friends and supporters out there.

Trac is not a web development framework, it is an application. Adding support for extra goodies is fine, but it should only result in changes to the core when there are general benefits as well. I'm sure people have similar feelings about us not supporting Mako as a template engine (though really you could as a plugin, fully in line with my point above), or not using CherryPy for request handlers. jQuery provides a noConflict mode so that it can easily be used on legacy-ish sites with existing code that uses $ for something, I see no reason that Prototype (or any other library) shouldn't do something similar. This would be exactly what we both want, a simple core that does what the developers want and the ability to extend it with other tools.

in reply to:  8 comment:9 by Noah Kantrowitz, 13 years ago

Replying to anonymous:

I should mention that is me.

comment:10 by rohitbrai@…, 13 years ago

And no one is using it here as a web development framework.
But everyone will agree that one size doesn't fit all. No one would use Vanilla trac out of the box without certain plugins. And plugins are not about web development, they are there to provide people what they need. For in our case a dashboard to give the project overview and simplified ticket creation and submission are must for it to be even considered by the developers and managers ad to our clients to whom we are referring trac a a solution
When I present a case for trac, I have to fight for it and tell that it can achieve the effect they want. For example no one following Scrum would have ever used it if it was not for the Scrum Plugin.
Yes, I agree prototype library should provide a compatibility/no-conflict mode, but they don't. I have already put a request there, but depending the amount of code and no of usages they have I doubt when it will be taken up. How it will be incorporated in their existing design and stuff.
While here in a sinle use case, we can easily accomodate it our code.
In all this discussion I don't understand one small thing, why do we have to say, if you live by our rules you are welcome else you are on your own.
You said it was not about ideology, it was about design and requires a lot of work, but the simplest solution is right there. One patch to search.js.
If any other plugin devleoper finds a conlict with someother js while developing his plugin he can upload a patch which can be included at that stage.
No one has to go in full time to find all usages of jquery and put them in no conlict mode. It's as-you-go and if someone decides to take out time and patch out every JS in trac, it' good.
But if it is just about sticking to your decision and asking people to obey/bind by it, and making strict rules around who and what you can contribute and use, then yes surely we must not at all support prototype or any other library and tell people to go and find out their own means.
Make it difficult for anyone who fails to obey to build/maintain/support any functionality with a fear of breaking compatibility with the main source.
I can imagine how painful it will be for poeple using my dashboard plugin to upgrade to the latest trac. the same download latest, patch latest install latest cycle. And may be that is sufficient for me as a message from "Trac Team" that "come on you are not using the lbrary we use/support, you can't exist, go clean up your code of anything hat uses prototype, learn jQuery, migrate what ever can be done with it to it, if it doesn't support that remove or redo the code and then you can talk"
Is it about politics of decision or control?

comment:11 by Christian Boos, 13 years ago

I wouldn't mind using jQuery in compatibility mode.

Global variables are just recipe for trouble, let alone a one character global variable that every toolkit feels cool to use…

Alternatively, can't we simply turn compatibility mode on when we're done with jQuery?

  • layout.html

     
    2626    <script type="text/javascript" src="${href.chrome('common/js/ie_pre7_hacks.js')}"></script>
    2727    ${Markup('&lt;![endif]--&gt;')}
    2828    ${select("*[local-name() != 'title']")}
     29    <script type="text/javascript">jQuery.noConflict();</script>
    2930  </head></py:match>
    3031
    3132  <div py:def="navigation(category)" id="${category}" class="nav">

(untested, please try it with your Prototype code)

Other alternatives: Referencing Magic - Shortcuts for jQuery

in reply to:  11 comment:12 by Christian Boos, 13 years ago

Follow-up to the above:

Alternatively, can't we simply turn compatibility mode on when we're done with jQuery?

Ok, just realized that most probably you load your Prototype javascript code by the way of a call to add_script, so the proposed change alone wouldn't do anything.

comment:13 by Christopher Lenz, 13 years ago

Component: search systemgeneral
Milestone: 0.11.10.11
Owner: changed from Jonas Borgström to Christopher Lenz
Status: newassigned

I'll look into moving our stuff to noConflict mode.

But be aware that Prototype isn't really a good choice for plugins. It has a number of side effects, extending "builtin" objects, which may break other code relying on the standard behaviour (for example, for (var i in myArray)). You've been warned :)

Also, I got to say that it sucks that we need to change our stuff because Prototype doesn't seem to think it's necessary to play well with others. But you say you've filed a ticket on their side, so let's see how that goes.

comment:14 by rohitbrai@…, 13 years ago

Glad to see that :)

I know protoype has a few effects and we have to be careful about using it, but there are some libraries that rely on Prototype, like Script.aculo.us and Xml.ObjTree and many others.
I also know that it seems wrong to have our code changed just to incorporate others into it, but as we say in India, when you adopt others you grow larger.
By supporting prototype as an example we show that we are willing to go out of the way to adopt others and we loudly say "We welcome all!!!"
May be not for others, for me it is very much ideological and symbolic :D

comment:15 by Christopher Lenz, 13 years ago

Where's the ticket you filed on Prototype?

I found this: http://dev.rubyonrails.org/ticket/9185

We don't support Prototype's use with other JavaScript frameworks at this time.

Way to go, guys</sarcasm>

by Christopher Lenz, 13 years ago

Attachment: jquery_noconflict.diff added

Make the Trac Javascript files not assume that the jQuery function is bound to the global $ name

comment:16 by Christopher Lenz, 13 years ago

I've attached a patch that changes all the Javascript files used by Trac so that they don't assume that jQuery is bound to the $ name. I think this should address your use case, and it'd be nice if you gave it a try.

This does not actually call jQuery.noConflict(). The reason is that if you're using Prototype in a plugin, the Prototype JS files will be included after the jQueryscript, and thus overwrite the $ function.

Also, the various templates still use $ directly. The assumption here is that if you're writing a plugin that uses Prototype, you'll not be rendering any of those templates, but your own.

comment:17 by Christopher Lenz, 13 years ago

Um, some feedback would be really nice here ;)

Anyway as this actually doesn't change anything in any significant way, and improves the modularity of our Javascript files (all jQuery plugins work this way, basically), I'll be checking this in in the next couple of days.

comment:18 by Noah Kantrowitz, 13 years ago

This seems like a decent compromise pending better conflict handling in other libraries.

comment:19 by rohitbrai@…, 13 years ago

Hey Friends, Am currently out of office and hence unable to look into this. Will be back on thursday and check this out. Sorry for the inconvinience.

comment:20 by Christian Boos, 13 years ago

Tomorrow it's Thursday again, you'll get a second chance to test the patch ;-)

comment:21 by rohtibrai@…, 13 years ago

Sorry friends, Am allocated to another project for now. And the work pressure here is a bit high. So I couldn't get a chance to check it out. In the meanwhile, I communicated with a few people, and they said Prototype will become compatible with other libraries as it is the part of Open Ajax Alliance.

Had told my friend working on it to, and he has told me it works. Sorry for not coming back to you in time.

I'll try to put up our dashboard code online too, so other can also use it, currently i have to get permission from the management.

Kind regards, Rohit Rai http://mytechrantings.blogspot.com

comment:22 by Christian Boos, 13 years ago

The patch doesn't work fully. Javascript functions created within the function($) { ... } (jQuery) block won't be reachable from outside unless attached to the jQuery object, which was not done for all such functions.

Also, we should perhaps add a trac_ prefix to such functions, e.g. $.fn.highlightText should become $.fn.trac_highlightText so that we can easily see what's coming from us.

by Christian Boos, 13 years ago

More complete version of the patch, with "trac_" scoped global functions.

comment:23 by Christian Boos, 13 years ago

I've updated the patch a bit to include the suggested changes. Everything seems to work, but more testing is probably needed.

Now I wonder whether the embedded Javascript code in html pages shouldn't be encapsulated as well?

in reply to:  22 ; comment:24 by anonymous, 13 years ago

Replying to cboos:

The patch doesn't work fully. Javascript functions created within the function($) { ... } (jQuery) block won't be reachable from outside unless attached to the jQuery object, which was not done for all such functions.

Which functions were those? I can't easily identify such cases in your patch.

Also, we should perhaps add a trac_ prefix to such functions, e.g. $.fn.highlightText should become $.fn.trac_highlightText so that we can easily see what's coming from us.

I don't agree with that, it's really ugly and IMHO not necessary. If we do need something like that, it should be $.trac.XXX.

comment:25 by Christopher Lenz, 13 years ago

(previous comment by me, sorry)

in reply to:  24 ; comment:26 by Christian Boos, 13 years ago

Replying to anonymous:

Replying to cboos:

The patch doesn't work fully. Javascript functions created within the function($) { ... } (jQuery) block won't be reachable from outside unless attached to the jQuery object, which was not done for all such functions.

Which functions were those? I can't easily identify such cases in your patch.

addAnchor, enableExpandDir, enableBlame, suggest. Plus all the backward compatibility functions in trac.js that simply need to be defined outside that scope.

Also, we should perhaps add a trac_ prefix to such functions, e.g. $.fn.highlightText should become $.fn.trac_highlightText so that we can easily see what's coming from us.

I don't agree with that, it's really ugly and IMHO not necessary. If we do need something like that, it should be $.trac.XXX.

Right, that's exactly why I didn't commit the patch ;-) However, I differ on the not necessary part: defining jQuery methods with such common names as enabled and checked looks particularly prone to collisions.

in reply to:  26 comment:27 by Christopher Lenz, 13 years ago

Replying to cboos:

However, I differ on the not necessary part: defining jQuery methods with such common names as enabled and checked looks particularly prone to collisions.

In any case that change has little to do with this ticket.

comment:28 by Paul Irish <paul.irishEWWSPAM@…>, 12 years ago

Hi Guys. I've done work before making jQuery code compatible with Prototype. I'd love to help get this code in order. If it's cool with you, I'll post a patch that'll straighten things out.

comment:29 by Christian Boos, 12 years ago

Yes, please do so.

in reply to:  28 comment:30 by anonymous, 12 years ago

Replying to Paul Irish <paul.irishEWWSPAM@gmail.com>:

Hi Guys. I've done work before making jQuery code compatible with Prototype. I'd love to help get this code in order. If it's cool with you, I'll post a patch that'll straighten things out.

Yes this would be VERY helpfull!!!

by Paul Irish <paul.irishEWWSPAM@…>, 12 years ago

Attachment: jquery-namespace.diff added

namespacing jQuery to $j, thus freeing up $ for Prototype

comment:31 by Paul Irish <paul.irishEWWSPAM@…>, 12 years ago

Just added a patch that should clear up the $ issues. I used jQuery.noConflict() to remap the object to $j, and then replaced all instances of $ to $j. Now whenever jQuery is used, it must be referenced with $j.

I will admit there is an opportunity to keep the $ while maintaining Prototype compatibility. Essentially it looks like:

jQuery.noConflict();

(function($) {
  $("textarea.wikitext").each(function() { addWikiFormattingToolbar(this) });
})(jQuery);

Note the use of $ in line 4. This is worth considering, but it raises some tricky scope issues that I couldn't answer without knowing this code a little better.

For now I'll suggest that the extra character $j isn't too bad a concession; let me know if anyone feels differently and I'll work on the different solution.

Btw- My dev environment is not up yet, so I made these changes blind. Since they're not very invasive, I'm fairly confident they're bug-free.

comment:32 by Christian Boos, 12 years ago

Well, looks like you didn't take a look at the previous patches on this ticket… we're clearly favoring the (function($) {...})(jQuery); approach. A good starting point is my patch (attachment:jquery_noconflict-r6245.diff), just remove all the trac_ renames which are unrelated (see comment:26).

The problem is that I haven't checked that patch when another Javascript library was used, so I don't know if all that needs to be done was actually done. Also, I don't know if we would need to also adapt the inlined use of jQuery in the templates themselves.

comment:33 by Paul Irish <paul.irishEWWSPAM@…>, 12 years ago

Gotcha okay. We can definitely go with that approach, when I originally looked at that diff there were a few parts that threw me off. Yes, in the templates all uses of $(document).ready() will need to be tweaked, too, but that's not too hard. I'll be on holiday for the next week and the plane takes off in a couple hours, but I'll post the corrected patch when I'm back!

by Paul Irish <paul.irishEWWSPAM@…>, 12 years ago

Attachment: jquery-safeify.diff added

new jquery/prototype compatibility script

comment:34 by Paul Irish <paul.irishEWWSPAM@…>, 12 years ago

A little earlier than expected (nothing like writing code on a beach in mexico!!)

Here's the patch using the technique described above. All scope issues should be accounted for: making all global functions defined globally, etc.

btw- dev env is up on my machine. everything looks smooth.

comment:35 by Christian Boos, 12 years ago

Resolution: fixed
Status: assignedclosed

… but it looks like it was applied a little later than expected (just to punish you for having had a great time coding this on the beach) ;-)

Great work, everything seems to be working fine.

in reply to:  35 ; comment:36 by Emmanuel Blot, 12 years ago

Replying to cboos:

… but it looks like it was applied a little later than expected (just to punish you for having had a great time coding this on the beach) ;-) Great work, everything seems to be working fine.

… except I think it has broken some plugins… It seems the changeset number where the patch has been applied is missing from this ticket.

in reply to:  36 comment:37 by Emmanuel Blot, 12 years ago

Replying to eblot:

It seems the changeset number where the patch has been applied is missing from this ticket.

Would be [6572]

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christopher Lenz.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christopher Lenz to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.