WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#15843 closed defect (bug) (invalid)

Fix dependencies so scripts are concatenated

Reported by: nacin Owned by:
Milestone: Priority: high
Severity: normal Version:
Component: External Libraries Keywords:
Focuses: Cc:

Description

The jQuery UI dependency tree is too complex for our dependencies package, which cause them to not concatenate into load-scripts.

We've fixed the side effects by wrapping things in document.ready and such.

We should try to fix this for 3.1 so we're not losing the load-scripts benefit.

Simple solution would be to simplify the dependencies. We can improve our code in a future release.

Change History (15)

comment:1 @westi4 years ago

I had a quick dig into this and we are not going to solve this easily.

I think this goes on the plate for looking at in 3.2

comment:2 @scribu4 years ago

  • Cc scribu added

comment:3 @scribu4 years ago

Could you describe the problem in more detail?

I'm only seeing two load-script.php calls: one for jquery,utils and another for all the rest, including the jQuery UI stuff.

Is the goal to have a single load-script.php call?

comment:4 @westi4 years ago

Go to Edit a Post.

You get in the Header:

<script type='text/javascript' src='http://trunk.domain/wp-includes/js/l10n.js?ver=20101110'></script>
<script type='text/javascript' src='http://trunk.domain/wp-admin/load-scripts.php?c=1&amp;load=jquery,utils,editor&amp;ver=151e6df801b09d30f43e1a8f2114dea7'></script>

In the Footer:

<script type='text/javascript' src='http://trunk.domain/wp-admin/load-scripts.php?c=1&amp;load=admin-bar,hoverIntent,common,jquery-color,schedule,wp-ajax-response,autosave,wp-lists,jquery-query,jquery-serialize-object,list-table,jquery-ui-core,jquery-ui-widget,jquery-ui-mouse,jquery-ui-resizable,admin-comments,suggest,jquery-ui-sortable,postbox,post,word-count,thickbox,media-upload&amp;ver=7f23cd0951f2123c9380a176139079da'></script>

<script type='text/javascript' src='http://trunk.domain/wp-includes/js/jquery/ui.draggable.js?ver=1.8.7'></script>
<script type='text/javascript' src='http://trunk.domain/wp-includes/js/jquery/ui.button.js?ver=1.8.7'></script>

<script type='text/javascript' src='http://trunk.domain/wp-includes/js/jquery/ui.position.js?ver=1.8.7'></script>
<script type='text/javascript' src='http://trunk.domain/wp-includes/js/jquery/ui.dialog.js?ver=1.8.7'></script>

All the jquery ui ones should be loaded by the footer load-scripts call.

comment:5 follow-up: @scribu4 years ago

Ok, thanks.

If this is not a regression, I'm for punting as well.

comment:6 in reply to: ↑ 5 @westi4 years ago

Replying to scribu:

Ok, thanks.

If this is not a regression, I'm for punting as well.

I think it is only a regression in so much as the script loader hasn't changed but the dependency tree we now have causes it issues.

comment:7 @nacin4 years ago

It is. Caused by the more complex dependencies in jQUI.

comment:8 @westi4 years ago

This is not a bug in the dependencies :-)

<script type='text/javascript' src='http://trunk.domain/wp-includes/js/jquery/ui.draggable.js?ver=1.8.7'></script>
<script type='text/javascript' src='http://trunk.domain/wp-includes/js/jquery/ui.button.js?ver=1.8.7'></script>
<script type='text/javascript' src='http://trunk.domain/wp-includes/js/jquery/ui.position.js?ver=1.8.7'></script>
<script type='text/javascript' src='http://trunk.domain/wp-includes/js/jquery/ui.dialog.js?ver=1.8.7'></script>
<script type='text/javascript' src='http://trunk.domain/wp-includes/js/tinymce/plugins/wpdialogs/js/popup.js?ver=20101119'></script>
  • wpdialogs depends on jquery-ui-dialog
  • jquery-ui-dialog depends on jquery-ui-resizable, jquery-ui-draggable, jquery-ui-button, 'jquery-ui-position

The script it not printed until late on until this call wp_print_scripts( array( 'wpdialogs-popup' ) );

comment:9 @westi4 years ago

That call is in wp_tiny_mce FYI

comment:10 @scribu4 years ago

Related: #9346

comment:11 follow-up: @nacin4 years ago

So, as westi found, it's not dependencies. #blamenacin

Two options for 3.1:

  • Leave this as is for 3.1. It doesn't look like anything is breaking. (But we should confirm.)
  • Enqueue earlier on the pages we need. If we don't need it (plugin actually not used, etc), then we don't need it and it's a little extra being included in the load-scripts call. Better than the five extra HTTP requests I think.

I like # 2, personally. Will look further.

comment:12 in reply to: ↑ 11 @westi4 years ago

Replying to nacin:

So, as westi found, it's not dependencies. #blamenacin

Two options for 3.1:

  • Leave this as is for 3.1. It doesn't look like anything is breaking. (But we should confirm.)
  • Enqueue earlier on the pages we need. If we don't need it (plugin actually not used, etc), then we don't need it and it's a little extra being included in the load-scripts call. Better than the five extra HTTP requests I think.

I like # 2, personally. Will look further.

It's not a serious bug and can wait for 3.2 IMHO

comment:13 follow-up: @westi4 years ago

Ideally we should come up with a way to enqueue tinymce via the script loaded in a separate group which we can then print out in a similar way to how it works now.

It could then depend on these extra scripts and when it was enqueued we wouldn't need to print stuff out as well.

comment:14 in reply to: ↑ 13 @nacin4 years ago

Replying to westi:

Ideally we should come up with a way to enqueue tinymce via the script loaded in a separate group which we can then print out in a similar way to how it works now.

It could then depend on these extra scripts and when it was enqueued we wouldn't need to print stuff out as well.

I agree with that.

I'm fine with a punt here (actually, I'd probably close as invalid and open a new ticket for TinyMCE) but I'm trying to remember what problems in core we had here.

comment:15 @nacin4 years ago

  • Milestone 3.1 deleted
  • Resolution set to invalid
  • Status changed from new to closed

Closing as invalid. Need a new ticket for 3.2. koopersmith has mentioned he's willing to work on splitting wp_tiny_mce.

Note: See TracTickets for help on using tickets.