Make WordPress Core

Opened 9 years ago

Closed 7 years ago

Last modified 7 years ago

#33365 closed defect (bug) (duplicate)

Plugins admin: plugin-install.js prevents custom thickbox implementations

Reported by: offereins's profile Offereins Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.2
Component: Plugins Keywords:
Focuses: javascript, administration Cc:

Description

In wp-admin/js/plugin-install.js all thickbox links on the current page are given specific width and height values to fit the screen nicely. In addition, all thickbox links within the plugin_row_meta are manipulated, assuming they all are the Plugin Details iframe popup.

When one wants to use a custom thickbox, either in the plugin_row_meta or elsewhere on the same page, it requires superfluous javascript to undo these manipulations. There is no easy way - or my js-skills are lacking - to do this. Targeting the proper links in the script, and not assuming all thickbox links are equal, would easily solve this issue.

I suggest to add a .plugin-details class to all thickbox links that open the Plugin Details popup, and target those links in the script with a.thickbox.plugin-details accordingly. This would count for the (all?) pages where the script is loaded, which are:

  • wp-admin\import.php
  • wp-admin\index.php
  • wp-admin\network\index.php
  • wp-admin\plugin-install.php
  • wp-admin\plugins.php
  • wp-admin\update-core.php

Attachments (2)

thickbox.patch (1.4 KB) - added by ccprog 8 years ago.
remove direct sizing of <div id='TB_ajaxContent'>
33365.patch (1.7 KB) - added by ccprog 8 years ago.
same content, but correct patch format

Download all attachments as: .zip

Change History (7)

#1 @DrewAPicture
9 years ago

@obenland: Mind weighing in here?

#2 @ccprog
8 years ago

Same is true for wp-admin/js/media-upload.js and its use for post.php / post-new.php

As a partial problem, function tb_position is overwritten to achieve responsive sizing of the box. In both cases, the new function assumes the existence of #TB_iframeContent, but not of #TB_ajaxContent. For the latter, the static sizing done by thickbox stands and breaks the layout on small screens.

So one solution would be to select both:

$('#TB_iframeContent, #TB_ajaxContent').width(...).height(...);

But then, I am wondering why thickbox sets these sizes for #TB_ajaxContent anyway. As far as I can see, setting the size is needed for iframes, and avoids width: 100% as a compatibility measure for IE6 (is there any other browser with a problem there? Must this still be supported?)

But for a simple <div id="TB_ajaxContent">, this is absurd. I'll attach a patch for wp-includes/js/thickbox/thickbox.js that removes the direct styling.

@ccprog
8 years ago

remove direct sizing of <div id='TB_ajaxContent'>

@ccprog
8 years ago

same content, but correct patch format

#3 @alanstorm
8 years ago

I ran into this, and other issue this week as well. The Wordpress core code's redefinition of tb_position and style changes can interfere with a plugin developer who wants to add a thickbox to certain admin pages. I've outline the problem in a Wordpress Stack Exchange Questions (http://wordpress.stackexchange.com/questions/235327/custom-thickbox-broken-on-dashboard-page), and also have a temp workaround in a self-answer (http://wordpress.stackexchange.com/a/235399/33566).

The workaround involves opening the thickboxes yourself with tb_show, and then using the following javascript function to un-do the Wordpress customizations that interfere with third party thickboxes. This code also tries to set Wordpress's customizations back after the thickboxes close. Not widely tested, but hope it helps point folks towards a soution

    var resetWordpressChangesToThickbox = function(){        
        //remove these styles from body if they exist
        var classes = ['about-php','plugin-install-php','import-php',
            'plugins-php','update-core-php','index-php'];
        var removed = [];
        $.each(classes, function(k,v){
            if(!$('body').hasClass(v)) { return; }
            removed.push(v);
            $('body').removeClass(v);
        });                

        var tb_position_original = window.tb_position;

        //some wordpress pages redefine this function which breaks
        //the thickbox, so we need to reset it here.  
        window.tb_position = function() {
            var isIE6 = typeof document.body.style.maxHeight === "undefined";
            jQuery("#TB_window").css({marginLeft: '-' + parseInt((TB_WIDTH / 2),10) + 'px', width: TB_WIDTH + 'px'});
            if ( ! isIE6 ) { // take away IE6
                jQuery("#TB_window").css({marginTop: '-' + parseInt((TB_HEIGHT / 2),10) + 'px'});
            }
        }

        var tb_remove_original = window.tb_remove;
        window.tb_remove = function()
        {
            $.each(removed, function(k,v){
                $('body').addClass(v);
                window.tb_position = tb_position_original;
            });
            tb_remove_original();
        } 
    }

#4 @Offereins
7 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

Fixed in [41356]. See #41417 (duplicate).

#5 @SergeyBiryukov
7 years ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.