When activating this theme u should have a jquery update solution option in theme settings using cdn.

I propose the noconflict solution and use the most recent jquery version on google cdn:
ajax.googleapis.com/ajax/libs/jquery/1.8.3/jquery.min.js

This feature would make the theme more dummy proof and will no cost alot of development time. I'm willing to write this

Files: 
CommentFileSizeAuthor
#32 1846736-bootstrap-remove_jquery_cdn-32.patch5.03 KBmarkcarver
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#31 1846736-bootstrap-remove_jquery_cdn-31.patch4.89 KBmarkcarver
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#6 cdn_solutions_1846736_v3_a.patch5.21 KBfrankbaele
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#6 cdn_solutions_1846736_v3_b.patch426 bytesfrankbaele
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#4 cdn_solutions_1846736.patch4.99 KBfrankbaele
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#2 cdn_solutions_1846736_v2.patch454 bytesfrankbaele
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#2 cdn_solutions_1846736.patch4.74 KBfrankbaele
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch cdn_solutions_1846736_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 cdn_solutions_1846736.patch4.73 KBfrankbaele
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Comments

frankbaele’s picture

Status:Active» Needs review
StatusFileSize
new4.73 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

I wrote a patch that adds cdn for jquery and css, can somboy help me test this more

frankbaele’s picture

StatusFileSize
new4.74 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch cdn_solutions_1846736_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new454 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

forgot the new js file added extra patch

Status:Needs review» Needs work

The last submitted patch, cdn_solutions_1846736.patch, failed testing.

frankbaele’s picture

StatusFileSize
new4.99 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
frankbaele’s picture

Status:Needs work» Needs review
frankbaele’s picture

StatusFileSize
new426 bytes
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
new5.21 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

forgot the situation where you want cdn js and dont want jquery cdn

wundo’s picture

Status:Needs review» Reviewed & tested by the community

+1 to committing this.

frankbaele’s picture

Status:Reviewed & tested by the community» Fixed

commited to the 2.x branch

cweagans’s picture

Why would you commit this!? Updating jQuery like that breaks a LOT of Drupal, which is the entire reason that we *have* jquery_update module. You have just destroyed any chance of being able to use this theme.

cweagans’s picture

And even if it doesn't break things, you're still loading two instances of jQuery on the page, which completely kills front end performance.

frankbaele’s picture

This option gives a quick solution for people with limited development knowledge, this feature isn't hardcoded. You can deactivate this in the settings.

cweagans’s picture

So you're giving people with limited development knowledge a switch that can, worst case, completely break their site, and best case, slow down frontend performance by a factor of two?

That doesn't seem wise.

Good documentation about how to download jquery_update and whatnot is the right path here.

frankbaele’s picture

1) how do you mean break there site? can you give an example how this commit will break their site.
2) yes this is a suboptimal solution, so is jquery_update that you are advocating here.

My personal experience with the module is that it breaks more then that it fixes. Has been 4-5 months since my last usages of it, but i remember well it breaks alot of the ctools stuff.

The goal of the commit was too make bootstrap 2.x work out of the box, if you don't like the no-conflict jquery cdn solution. You can disable it and implement your own solution of choice, no performance drawback.

cweagans’s picture

Well the entire reason for jquery_update existing is that newer jQuery versions don't play nicely with core/contrib Javascript. Sure, it breaks ctools stuff, but that's what happens when you mess with the jQuery that's being loaded: things break. jquery_update aims to fix that by swapping out the broken javascript files with fixed versions. Dropping some external jQuery in without fixing those files will cause them to break.

However, since you've added the external jQuery in noconflict mode, that /should/ be avoided, in theory. But that doesn't solve the problem of having two separate jQuery instances loaded on the page.

I think that it's totally reasonable to depend on jquery_update for this theme and not try to duplicate that functionality. Let jquery_update handle the intricacies of updating jQuery and focus on what this theme is trying to do: integrate Bootstrap.

I have no issue with externally hosted resources, but pulling in jQuery like that is reckless. jquery_update is the de-facto, community accepted solution. Going against that is causing more problems than it solves.

frankbaele’s picture

I still don't get why you have a problem with this, we didn't choose any solution, jquery_update or no-conflict. I Just made one of the solutions easier too use.
You can't expect that everybody on drupal.org wants to use the jquery_update solution but they are stuck with it because off there limited skills. I just added an other solution to the problem that is also easy to use.

Ok maybe i should add some more disclaimer and explain what the consequences can be of the solution. So that the user is more warned. But you can't just kill off this idea because it doesn't follow the community accepted solution.

frankbaele’s picture

Status:Fixed» Needs work
frankbaele’s picture

Status:Needs work» Active
cpliakas’s picture

A big +1 to cweagans' concerns.

I totally understand the desire to make things easier for the site builder by providing a quick switch, however it does give inexperienced people a lot of rope to hang themselves with as well. Having worked as a consultant delivering Drupal trainings to people who are inexperienced, experienced, and everywhere in between, the quickest way to get them to passionately hate a technology is to provide them with an easy button loaded with gotchas.

Specifically, the majority of JavaScript in Drupal assumes the version of jQuery that ships with core, which I believe is 1.4. If you upgrade to 1.7, a lot of things break such as the ability to use certain features in Views, the core ajax functionality in some cases, and a bunch of other bugs that place you deep in the guts of jQuery and core Drupal JavaScript to debug. So although the original intent was to help out inexperienced developers, the result is that they are thrown head first into debugging that even Drupal wizards find difficult.

At the end of the day, no matter how inexperienced you are, you should be able to install a module if you are working with Drupal. Heck, you already installed this theme, which is arguably more difficult to do. The benefit of piggy-backing off of jQuery Update is that the module is working to address the incompatibilities for you, so even though there are still gotchas, there is a large community of people actively working to resolve the problems in the module without the site builder or this theme having to address them. jQuery Update is a top 30 module in terms of usage, and that's saying something. Let's do site builders a favor and guide them towards the best solution available that is backed and tested by a large community.

Thanks in advance for your consideration to this,
Chris

frankbaele’s picture

hi thx for the rply cpliakas,

I totally agree with you on the first part, it is very dangerous to add these quick switches that can backfire easily.

But the second part kinda confuses me because its not related to the solution presented in this patch/commit. The no-conflict solutions doesn't replace jquery system wide, it just creates as figure of speech a small bubble where you can run a different version of jqeury and jqeury code without conflicting with the old existing version and code. The only downside of this solutions is that you are loading 2 versions of jquery on your website. This technique is also mentioned in: http://drupal.org/node/1058168

Also i'm not advocating for one solution, the jquery_update module does a fine job for the difficulties it challenges. But it maybe not the solution you always want to use. So the only thing i'm giving to the user of the theme is a choice too use the no-conflict solution without programming or installing an extra module. This extra lines of code don't have a performance impact, they don't ruin the theme as mentioned by some. They just add an option that can be ignored, no assumptions, no duplicate code or functionality.

As mentioned before, there needs to be better documentation if the feature stays and it shouldn't be enabled out of the box.

frank

cpliakas’s picture

But the second part kinda confuses me because its not related to the solution presented in this patch/commit.

OK, I see now. Thanks for sticking with me on this. I think I missed the overall point and honestly don't have any expertise in the no-conflict stuff, especially with the multiple version of jQuery on the same page. So given that this topic is about something different than I though it was and something that I have little value to offer, kindly ignore me. :-)

Thanks,
Chris

cweagans’s picture

Yes, but executing two separate instances of jQuery on the same page is a huge performance issue. The jquery_update module is the solution that you should use. Period. If you're not doing that, then you're causing more headaches than you're solving. Anyone should be able to download a module if they have already downloaded a theme.

jquery_update is *the* accepted way to update jquery mostly due to the community of contributors that are working on fixing the problems that it causes. Loading another jQuery in noconflict mode is a bad idea, especially when it's done by an option that beginners who don't know any better will use. You should be steering said beginners toward smart solutions, not quick solutions that cause performance issues.

cpliakas’s picture

FWIW, I actually just ran into an issue where having two versions of jQuery breaks the Bootstrap JavaScript APIs. This is confirmed at https://groups.google.com/d/msg/twitter-bootstrap-stackoverflow/cMv8WYCd....

cpliakas’s picture

natted’s picture

Sorry I missed this discussion earlier. Just to offer another suggestion / perspective.

We could possibly look to redevelop the old twitter_bootstrap_ui module ( http://drupal.org/project/twitter_bootstrap_ui ) and recommend it as a supported solution.

If we did so, then we could offer:

- Libraries API support #1852332: Integrate with the Libraries API module
- Ensure that jquery_update is set as a dependency (leveraging jquery_update rather than hotlinking via CDN directly)

We could also make the "bootstrap_ui" module set the jQuery version (using jquery_update) on installation....

frankbaele’s picture

danillonunes’s picture

Besides the issues reported by cweagans, this feature, due to the order of how the cdn script is loaded, breaks once, form, cookie and basically every jQuery plugin that is loaded before the theme layer (aka every jQuery plugin which is included by a Drupal module, even the Core ones).

boban_dj’s picture

@cweagans
As a beginner i had some issues,
Here some issues I had with jQuery update module:

Bootstrap at least needs version 1.7 (jQuery update module.
If choosing 1.8, my Views module breaks completely(version = "7.x-3.5")

jQuery update module version 1.7 --> Beauty Tips module (version = "7.x-2.0-beta2")
the user-interface is not working properly(no hover examples and the settings don't work).
With version 1.5 jQuery Update it does work.

For me it is not clear if the no conflict solution for multiple jQuery libraries works and how to implement and manage them.
jQuery update module works well, but to have checking all contrib modules if they work with it, is too much, as I need to work with Bootstrap as well.

I need to have the possibility to implement modules like Beauty tips. It looks now more easier to implement them in my custom small beginners module, and add the jQuery myself.

I think the theme should be separated from the jQuery integration in Drupal, as it has the drupal behaviors already set up as the architecture for this. Just loading some new version through the theme setting I agree is like an elephant in a porcelain shop.

If I need to clarify some problems I had regarding specific issues, please let me know I will provide more info.

markcarver’s picture

I couldn't agree more with @cweagans and @cpliakas. This patch should be removed.

From http://api.jquery.com/jQuery.noConflict/:

Many JavaScript libraries use $ as a function or variable name, just as jQuery does. In jQuery's case, $ is just an alias for jQuery, so all functionality is available without using $. If you need to use another JavaScript library alongside jQuery, return control of $ back to the other library with a call to $.noConflict(). Old references of $ are saved during jQuery initialization; noConflict() simply restores them.

If for some reason two versions of jQuery are loaded (which is not recommended), calling $.noConflict(true) from the second version will return the globally scoped jQuery variables to those of the first version.

To summarize, clarify and reiterate what is stated on jQuery's documentation: using jQuery.noConflict() is not meant for loading two versions of jQuery. It is really only for special use cases where another library (like MooTools) is being used and the $ variable needs to be reassigned.

I understand that this patch was originally committed to help novice users implement a Bootstrap theme in a rather quick fashion with minimizing necessary understanding of why a higher version of jQuery is needed. However, I also think this is a very bad approach. This presents several issues:

  1. This "enhancement" is duplicating code & functionality. This inadvertently forces support of jQuery in this project's issue queue. The jquery_update module exists for a reason, it should be used.
  2. As previously mentioned, this method of implementation also forces two versions of jQuery to be loaded. Not only is this frowned upon by jQuery's documentation itself, it also adds a significant and an unnecessary impact on front-end performance.
  3. Despite the attempt to help simplify getting Bootstrap "working", this actually provides more confusion in the long run. By loading two separate versions of jQuery, this actually makes it more difficult to track down bugs. Does it lie with the the jQuery version loaded with the Bootstrap base theme? Is it caused by a contrib module? Are they somehow intertwined? These are some of the questions the "novice" user might ask, if not more.
  4. Instead of coding to standards and making things more solid, we're going down the path of providing "dummy" support. Instead of providing necessary documentation on how to get things working probably, we're introducing yet another path of uncertainty. How does this help a novice? How does this help reduce the amount of issues in the queue?
  5. Drupal has never been synonymous with the words "dummy proof". Drupal is inherently complex, especially when you start getting into the front-end aspects. Now I'm not saying we shouldn't help simplify things so they're easier for novice users. However, documentation is the necessary path, not adding unnecessary code.
  6. @boban_dj about Views and 1.8: this issue has been known for a while and was finally committed: #1802198-27: Views UI breaks with jQueryUI 1.8.2. I would like to point out though, that this is how issues should be handled. When a contrib module breaks because of a jQuery update, then it should be addressed and fixed by that module. We really should not get in the habit of trying to load two versions of jQuery to avoid fixing issues. Things break, let's fix them... not cover them up.
markcarver’s picture

Title:Integrate a optional jquery solution using cdn» Remove jQuery CDN setting in favor of using jQuery Update module

Changing title since the patch was already committed.

wundo’s picture

+1 to removing it too, jQuery Update is a better option IMHO

markcarver’s picture

Status:Active» Needs review
StatusFileSize
new4.89 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
markcarver’s picture

StatusFileSize
new5.03 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Attaching revised patch, forgot to take out the commented legacy exclude of misc/jquery.js in the .info file.

wundo’s picture

Status:Needs review» Fixed

Committed.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.