View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0020081 | mantisbt | javascript | public | 2015-09-07 10:37 | 2015-12-06 02:45 |
Reporter | dregad | Assigned To | dregad | ||
Priority | normal | Severity | major | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.3.0-beta.3 | ||||
Target Version | 1.3.0-rc.1 | Fixed in Version | 1.3.0-rc.1 | ||
Summary | 0020081: Moving JS to bottom is causing 'ReferenceError: jQuery is not defined' errors | ||||
Description | Implementation of 0013285 is causing a regression. The Snippets plugins [1], which atrol and I heavily use for user support, no longer works. The Javascript log shows the following errors: <pre> | ||||
Steps To Reproduce |
| ||||
Tags | No tags attached. | ||||
The problem lies with EVENT_LAYOUT_RESOURCES plugin event, which is triggered at the end of HTML head. The plugin relies on jQuery, but its own scripts are loaded before the library, which causes the error. I'm not sure what the best way to fix this would be. One approach could be to use a distinct event for plugins to load javascript resources (e.g. EVENT_LAYOUT_RESOURCES_JAVASCRIPT) after jQuery's been loaded in html_body_end(). |
|
Furthermore, the event reference is now in contradiction with what the core actually does: https://mantisbt.org/docs/master/en-US/Developers_Guide/html-desktop/#dev.eventref.output.layout EVENT_LAYOUT_RESOURCES (Output) |
|
EVENT_LAYOUT_RESOURCES is a valid event event and should be used by plugins to link CSS & JS files. The problem here is that the plugin has a dependency on JQuery. The plugin made an assumption about the presence of JQuery and when it is going to be loaded in the page. That's is the problem. There are few ways to fix this: (1) Move base JS libraries to <head> - backward compatible (2) EVENT_LAYOUT_BODY_END |
|
Thanks for looking into this.
Correct. In 1.2, jQuery was a plugin of its own, so dependency was easily managed by making sure the jQuery plugin was loaded first.
Again, correct. With 1.3, since jQuery is part of core, plugins should be safe to assume that the library is loaded for them when the documentation tells them they can load their resources.
Keep in mind that plugins do not have so many options to load resources since they must rely on available events as provided by Core. As per documentation, EVENT_LAYOUT_RESOURCES was the right way to do things, until 0013285. For the record, I am not saying that moving JS to the bottom was bad or wrong, or that the plugin should not adapt to the new method. Now with regards to your proposed options:
IMO, this option implies fully reverting 0013285. Doing things half-way (i.e. only moving jQuery back up) doesn't make sense, as sooner or later we'll face the same issue with another plugin which relies on jQueryUI.
This event currently is not placed in the right location: it is triggered before jQuery is loaded. This implies either
After that, all plugins relying on jQuery or jQueryUI need to be adapted to use the new event.
I assume you're basing your recommendation on this Yahoo developers article [1]. I'd like to challenge that, and point out another argument which goes the other way, i.e. it's better to have a page that appears to load slightly more slowly, than a page that appears broken [2]. Based on the above, my personal choice would be option 1, i.e. move jQuery/jQueryUI back to the top. [1] https://developer.yahoo.com/performance/rules.html |
|
Link to @syncguru's PR: https://github.com/mantisbt/mantisbt/pull/642 @dregad The article you referenced makes sense. This would have been a good discussion as part of implementing 0013285. I suggest we start off with the implementation that @syncguru provided in his PR which should fix the plugins. If we notice issues coming in about app being broken and as we increase our dependency on Javascript, we can revert 0013285 and move the JS back to head. |
|
Indeed... I only just found it while researching a bit on this so-called "best practice", sorry ;)
Actually it would not I'm afraid, at least not without altering the plugins' code to load javascript resources on EVENT_LAYOUT_BODY_END instead of EVENT_LAYOUT_RESOURCES. I would therefore like us to complete the discussion here before changing plugins, as I'd rather avoid having to revert them after reverting 0013285 if it comes to that. |
|
@dregad - the excerpt below is from document [2] "Summarising, it really depends on what you’re making. If you’re still stuck in the “content” world then it’s maybe best to place your JavaScript at the bottom of the page. But if you are making web software you’re much better, placing JavaScript at the top, because functionality is the new king." I totally agree with the author assessment. For heavy JS app, most of the code will live in the JS thus there is no point in putting JS at the bottom. If you think about modern Backbone or Angular apps, there is really nothing in the page except that links to JS & CSS files. Mantis is largely a content app. The JS is very lightly used (if any in most pages). So putting the JS at the bottom for MantisBT aligns well with the author argument (and possibly yours) |
|
Good point @dregad. We should also based on this discussion update the documentation of the events in the developer manual to clarify our recommendation for plugin authors. |
|
You're right of course, but we also have to consider where we'll be in a year or two. Mantis is an application that was designed 15 years ago. Until very recently, it was even a system requirement to be able to fully operate without relying on any javascript at all. We are slowly moving away from that; starting with 1.3, javascript will no longer be optional. With 2.0 and the new UI, I'm sure we will rely more and more on dynamic contents. So even though we will most likely never become a "heavy JS app", we should ask ourselves whether (and by how much) our reliance on javascript will increase in the coming years. Of course, if we persist in loading javascript at page bottom, we need to update doc (and also make this change clear in release notes to the attention of plugin authors so they can update their code) |
|
One more issue introduced by 0013285, see 0020088. |
|
With this additional issue, I think the best course of action at this time is to revert. |
|
Just a thought from a humble developer, and I apologize if this has been mentioned before. It is good that jQuery is being included in the installation. However, jQuery constantly updates. Wouldn't it make more sense to keep it as a plugin, just like the Formatting or Graphs plugins? or even make it a core one? This would make it easier for developers to update the version if desired, without having to look in the mantis system directories. |
|
+1 for reverting. @bkraul, upgrading jquery whether it is in a plugin or not, is pretty much the same. It is currently a dependency for core and is treated in a consistent way with the rest of the libraries that we depend on. |
|
I reverted the change. For the record, this also impacted the source integration plugin's search page. |
|
MantisBT: master 8d7ea37e 2015-09-11 06:03 Details Diff |
Revert move-js-to-footer This change (see issue 0013282) introduced several regressions. This reverts commit 2dbc86b10d10495f1312b781fe0a3519a0d7152a, reversing changes made to aa9b4f8ad7908325633542840805c4c854adb1f1. Fixes 0020081, 0020088 |
Affected Issues 0013285, 0020081, 0020088 |
|
mod - core/html_api.php | Diff File |