WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 12 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 5 years ago.
14157-script-loader.diff (34.7 KB) - added by wonderboymusic 14 months ago.
14157-ms-php.diff (398 bytes) - added by voldemortensen 12 months ago.
Fixing hardcoding in wp-admin/includes/ms.php

Download all attachments as: .zip

Change History (33)

comment:1 @steak5 years ago

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

comment:2 @scribu5 years ago

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

comment:3 @scribu5 years ago

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

@wojtek.szkutnik5 years ago

comment:4 @wojtek.szkutnik5 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?

comment:5 @wojtek.szkutnik5 years ago

  • Keywords gsoc added

comment:6 @anantshri4 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.

comment:7 @nacin20 months ago

  • Component changed from General to Bootstrap/Load

comment:8 @wonderboymusic14 months ago

In 28903:

Use the WPINC constant when loading class-phpass.php

Props wojtek.szkutnik
See #14157.

comment:9 @wonderboymusic14 months ago

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

comment:10 @wonderboymusic14 months ago

In 28904:

WP_Filesystem_Base->abspath() should use the WPINC constant

Props wojtek.szkutnik
See #14157.

comment:11 @wonderboymusic14 months ago

In 28905:

wlwmanifest_link() should use the WPINC constant

Props wojtek.szkutnik
See #14157.

comment:12 follow-up: @nacin14 months ago

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

comment:13 @wonderboymusic14 months ago

In 28906:

Admin screens should use the WPINC constant

Props wojtek.szkutnik
See #14157.

comment:14 @wonderboymusic14 months ago

In 28907:

WP_Scripts->in_default_dir() should use the WPINC constant

Props wojtek.szkutnik
See #14157.

comment:15 in reply to: ↑ 12 @SergeyBiryukov14 months 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.

comment:16 @wonderboymusic14 months ago

In 28908:

Use includes_url() in wlwmanifest_link().

Props nacin.
See #14157.

comment:17 @wonderboymusic14 months ago

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

comment:18 @wonderboymusic14 months 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.

comment:19 @wonderboymusic14 months ago

#28686 was marked as a duplicate.

comment:20 @nacin14 months 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.

comment:21 @wonderboymusic14 months ago

In 28914:

Revert [28911] for performance concerns.

See #14157.

comment:22 @SergeyBiryukov14 months 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.

comment:23 @johnbillion14 months ago

  • Keywords has-patch removed

comment:24 @johnbillion14 months ago

  • Milestone changed from 4.0 to Future Release

comment:25 @SergeyBiryukov14 months ago

What's left to do here?

comment:26 @lancewillett13 months 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.

comment:27 @SergeyBiryukov12 months ago

#29335 was marked as a duplicate.

@voldemortensen12 months ago

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

comment:28 @voldemortensen12 months ago

  • Keywords has-patch added

comment:29 @wonderboymusic12 months ago

  • Milestone changed from Future Release to 4.1

comment:30 @wonderboymusic12 months 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.

Note: See TracTickets for help on using tickets.