Remove html_head_javascript() and relocate js to bottom of the page to speed up page rendering #605
Conversation
| foreach ( $g_scripts_included as $t_script_path ) { | ||
| html_javascript_link( $t_script_path ); | ||
| } | ||
|
|
||
| event_signal( 'EVENT_LAYOUT_BODY_END' ); |
There was a problem hiding this comment.
Should the body end event proceed the javascript, so the javascript is right before the closing body tag?
|
The title of the commit is too long. The issue title is more compact and I think conveys the message. We try to have commit titles be 50 characters or less. Otherwise 👍 with some minor comments. |
|
Are we sure that this really has no side effects ? Are all of the cases indeed covered by $document.ready() ? I'm concerned about the jscalendar (we really need to replace this thing at some point in the near future !), and also what about plugins ? They could be using require_js() too (didn't check) |
|
From Best Practices for Speeding Up Your Web Site Put Scripts at the Bottom In some situations it's not easy to move scripts to the bottom. If, for example, the script uses document.write to insert part of the page's content, it can't be moved lower in the page. There might also be scoping issues. In many cases, there are ways to workaround these situations. An alternative suggestion that often comes up is to use deferred scripts. The DEFER attribute indicates that the script does not contain document.write, and is a clue to browsers that they can continue rendering. Unfortunately, Firefox doesn't support the DEFER attribute. In Internet Explorer, the script may be deferred, but not as much as desired. If a script can be deferred, it can also be moved to the bottom of the page. That will make your web pages load faster. |
|
For the record: I'm not saying this shouldn't be done, I just want to make sure we're not introducing any regressions. |
|
I don't have a position either, just posted the reference. I suggest we come up with the list of features that we want to make sure work properly:
@dregad anything else you suggest checking? |
|
@dregad the issues that result from moving JS files or changing their loading order usually too severe to go unnoticed, that said, a quick testing will reveal problems quite easily. As such I don't consider this change to be risky. |
|
Not sure what you did with the pull request here, but I suggest you rebase the branch on top of latest master and push it again. |
Fixes #13285: Move script inclusions from HEAD to document footer
|
Not sure what went down either. Rebased the branch... should be good now. |
|
Any reason why this change is not merged yet? |
|
No objections from me, as long as the
has actually been performed... |
|
Yes, this was tested. I need to update this PR to resolve conflicts though. |
…g13285-move-js-to-footer # Conflicts: # core/html_api.php
|
👍 |
This is Rafik's original branch rebased on master. Pull request #605
|
Merged. Thanks Rafik ! |
Fixes #13285: Move script inclusions from HEAD to document footer