WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 2 years ago

Last modified 8 months ago

#14157 closed enhancement (fixed)

wp-includes references should be wiped off

Reported by: steak Owned by: wonderboymusic
Milestone: 4.1 Priority: lowest
Severity: minor Version:
Component: Bootstrap/Load Keywords: has-patch
Focuses: Cc:

Description

I think all references to wp-includes should be wiped off and use the WPINC WP constant. This may allow for a future renaming of the wp-includes folder if needed.

E.g in script-loader.php:

$scripts->add( 'jquery-ui-core', '/wp-includes/js/jquery/ui.core.js', array('jquery'), '1.7.1' );

To:

$scripts->add( 'jquery-ui-core', '/' . WPINC. '/js/jquery/ui.core.js', array('jquery'), '1.7.1' );

Attachments (3)

14157.diff (45.7 KB) - added by wojtek.szkutnik 6 years ago.
14157-script-loader.diff (34.7 KB) - added by wonderboymusic 2 years ago.
14157-ms-php.diff (398 bytes) - added by voldemortensen 2 years ago.
Fixing hardcoding in wp-admin/includes/ms.php

Download all attachments as: .zip

Change History (34)

#1 @steak
6 years ago

  • Summary changed from wp-include references should be wiped off to wp-includes references should be wiped off

#2 @scribu
6 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Priority changed from normal to lowest

#3 @scribu
6 years ago

  • Keywords needs-patch added; wp-includes WPINC removed

#4 @wojtek.szkutnik
6 years ago

  • Cc wojtek.szkutnik@… added
  • Keywords has-patch needs-testing dev-feedback added; needs-patch removed

Patch attached. Although, I have a few questions:
wp-admin/includes/update-core.php - it is unclear to me, if the wp-includes references should stay or not

wp-admin/network.php and wp-includes/rewrite.php I need strong testing and feedback on these rewrites, can anyone confirm it works as expected?

wp-includes/js/thickbox/thickbox.js two references to wp-includes in a js file, how can these two be handled?

#5 @wojtek.szkutnik
6 years ago

  • Keywords gsoc added

#6 @anantshri
5 years ago

  • Cc anantshri added

Hi This should be done.... if no one is working on it i can try creating a patch, as per my initail scan of the whole folder structure only 1-2 files can have some problem other then that all could be modified.

will start working if no objection is recieved.

#7 @nacin
3 years ago

  • Component changed from General to Bootstrap/Load

#8 @wonderboymusic
2 years ago

In 28903:

Use the WPINC constant when loading class-phpass.php

Props wojtek.szkutnik
See #14157.

#9 @wonderboymusic
2 years ago

  • Keywords needs-testing dev-feedback gsoc removed
  • Milestone changed from Future Release to 4.0

#10 @wonderboymusic
2 years ago

In 28904:

WP_Filesystem_Base->abspath() should use the WPINC constant

Props wojtek.szkutnik
See #14157.

#11 @wonderboymusic
2 years ago

In 28905:

wlwmanifest_link() should use the WPINC constant

Props wojtek.szkutnik
See #14157.

#12 follow-up: @nacin
2 years ago

[28905] is using WPINC directly in a URL, versus a filesystem path. It should instead be using includes_url().

#13 @wonderboymusic
2 years ago

In 28906:

Admin screens should use the WPINC constant

Props wojtek.szkutnik
See #14157.

#14 @wonderboymusic
2 years ago

In 28907:

WP_Scripts->in_default_dir() should use the WPINC constant

Props wojtek.szkutnik
See #14157.

#15 in reply to: ↑ 12 @SergeyBiryukov
2 years ago

Replying to nacin:

[28905] is using WPINC directly in a URL, versus a filesystem path. It should instead be using includes_url().

Also, string concatenation should probably not be replaced with commas, it seems less readable.

#16 @wonderboymusic
2 years ago

In 28908:

Use includes_url() in wlwmanifest_link().

Props nacin.
See #14157.

#17 @wonderboymusic
2 years ago

14157-script-loader.diff uses includes_url( ..., 'relative' ) all over script-loader.php

#18 @wonderboymusic
2 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 28911:

Use includes_url( ..., 'relative' ) in script-loader.php in lieu of hard-coding /wp-includes/.... everywhere.

Fixes #14157.

#19 @wonderboymusic
2 years ago

#28686 was marked as a duplicate.

#20 @nacin
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[28911] should be reverted. This is much slower, and the script loader has an internal implementation where it detects /wp-includes/ or /wp-admin/ at the start of a script and then switches it out with a base url. Some other checks specifically depends on wp-includes and wp-admin.

#21 @wonderboymusic
2 years ago

In 28914:

Revert [28911] for performance concerns.

See #14157.

#22 @SergeyBiryukov
2 years ago

[28911] also broke wp-admin/load-styles.php, which calls wp_default_styles() without loading wp-includes/link-template.php, so includes_url() is unavailable there. See #28686.

#23 @johnbillion
2 years ago

  • Keywords has-patch removed

#24 @johnbillion
2 years ago

  • Milestone changed from 4.0 to Future Release

#25 @SergeyBiryukov
2 years ago

What's left to do here?

#26 @lancewillett
2 years ago

Found a case in wp-admin/includes/ms.php: https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/ms.php#L810

var tb_pathToImage = "../../wp-includes/js/thickbox/loadingAnimation.gif";

Should be changed to:

var tb_pathToImage = '<?php echo includes_url( 'js/thickbox/loadingAnimation.gif' ); ?>';

Not sure if the relative context as second argument is needed.

#27 @SergeyBiryukov
2 years ago

#29335 was marked as a duplicate.

@voldemortensen
2 years ago

Fixing hardcoding in wp-admin/includes/ms.php

#28 @voldemortensen
2 years ago

  • Keywords has-patch added

#29 @wonderboymusic
2 years ago

  • Milestone changed from Future Release to 4.1

#30 @wonderboymusic
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 29741:

Make the URL output in _thickbox_path_admin_subfolder() use includes_url().

Props voldemortensen.
Fixes #14157.

#31 @johnbillion
8 months ago

#35825 was marked as a duplicate.

Note: See TracTickets for help on using tickets.