WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#17050 closed defect (bug) (fixed)

Built in Thickbox close image relative path error

Reported by: maorb Owned by:
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.1
Component: External Libraries Keywords:
Focuses: Cc:

Description

When using the built in Thickbox modal window on the frontned, the link to the close image is a relative path to - ../wp-includes/js/thickbox/tb-close.png

This making the icon not appear if user is on an inside page of the site, since the relative path is no longer correct.

The file contains this is wp-includes/js/thickbox/thickbox.js

Lines 8-13 are these:

if ( typeof tb_pathToImage != 'string' ) {
	var tb_pathToImage = "../wp-includes/js/thickbox/loadingAnimation.gif";
}
if ( typeof tb_closeImage != 'string' ) {
	var tb_closeImage = "../wp-includes/js/thickbox/tb-close.png";
}

When replaced with relative path to root and not to parent directory, it is working well and fixes this error --

if ( typeof tb_pathToImage != 'string' ) {
	var tb_pathToImage = "/wp-includes/js/thickbox/loadingAnimation.gif";
}
if ( typeof tb_closeImage != 'string' ) {
	var tb_closeImage = "/wp-includes/js/thickbox/tb-close.png";
}

Change History (9)

comment:1 @dd324 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

You cant use absolute paths (relative to root) because not all WordPress installs are rooted at /

The thickbox script contains paths designed for use within the admin area, If you want to use it on the front end you need to add something similar to the following to your head:

<script type="text/javascript">
//<![CDATA[
var tb_pathToImage = "<?php echo site_url('/wp-includes/js/thickbox/loadingAnimation.gif'); ?>";
var tb_closeImage = "<?php echo site_url('/wp-includes/js/thickbox/tb-close.png'); ?>";
//]]>
</script>

See #6453

comment:2 @dd324 years ago

  • Milestone set to Future Release
  • Resolution duplicate deleted
  • Status changed from closed to reopened

However, Now that I've closed it, I'm wondering why this isn't being handled by passing the url through site_url() + inserting it through our translation functions. This would allow it to work seamlessly on both the front and back ends..

comment:3 @maorb4 years ago

@dd32 Thanks and I agree with you.
Meanwhile, I will use your solution of adding the js vars to the head, it will be better than changing the core.

comment:4 @dd324 years ago

  • Milestone changed from Future Release to 3.2

comment:5 @dd324 years ago

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

(In [17596]) Pass image URL's to thickbox via L10n in script-loader.php. Allows thickbox to be used on any page in the install without defining the image URL variables. Fixes #17050

comment:6 @nacin4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:7 @nacin4 years ago

(In [17608]) Revert [17596] for now. props kawauso, see #17057, see #17050.

comment:8 @dd324 years ago

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

(In [17609]) Revert [17608] adding back [17596] and actually include the dummy function this time. See #17057 Fixes #17050

comment:9 @azaozz4 years ago

Perhaps we should (finally) separate the default scripts and default stylesheets from script loader and remove all these dummy functions. Should have done that when introducing load-scripts.php and load-styles.php.

That would probably make life a bit easier for caching plugins too.

Of course we will still need 1-2 dummy functions in these files when using a function directly in the JS args like in this case but that seems quite easier to manage and could be triggered by checking whether ABSPATH is defined, etc.

Note: See TracTickets for help on using tickets.