[wp-trac] [WordPress Trac] #17249: thickbox modal window dimensions are fixed in wp-admin

[WordPress Trac]

#17249: thickbox modal window dimensions are fixed in wp-admin
----------------------------+------------------------------
 Reporter:  DreadLox        |       Owner:
     Type:  defect (bug)    |      Status:  new
 Priority:  normal          |   Milestone:  Awaiting Review
Component:  Administration  |     Version:  3.1
 Severity:  normal          |  Resolution:
 Keywords:                  |
----------------------------+------------------------------
Changes (by chrisbliss18):

 * cc: gaarai@… (added)


Comment:

 I'd like to start a dialogue about this issue as it has plagued me for
 years. I'm sick of telling people that our theme or plugin has a
 compatibility problem with a plugin simply because the conflicting plugin
 enqueues media-upload on every admin page.

 The core issue is that any page that has media-upload enqueued on it will
 cause every Thickbox on that page to be modified to work as the media
 uploader expects the Thickbox to be modified, even if nothing on the page
 uses the media uploader.

 There are three critical issues that need to be resolved in order to fix
 this problem:

 * Rather than writing a custom handler to resize the media uploader's
 Thickbox, the media-upload script simply replaces the tb_position function
 that is defined by the Thickbox code. This means that the original sizing
 functionality offered by the Thickbox library will be completely replaced
 with a new one, causing drastically different Thickbox behaviors depending
 on whether or not the media-upload script was enqueued. While this may be
 technically elegant, it creates a huge number of limitations and potential
 conflicts when using WordPress' built-in Thickbox library.
 * The media-upload script automatically replaces all width and height
 arguments of any Thickbox link with its own calculated values. Even if
 there were a simple way to avoid the issue of the tb_position override,
 this code would still cause significant changes to any Thickbox link,
 resulting in unexpected behavior.
 * The media-upload script binds a call to the tb_position function to the
 window's resize event. This makes it very, very difficult to code around
 the issues created by the media-upload script.

 There are many possible solutions to this issue, but given that I'm very
 confused on why the script does certain things (such as why it changes all
 the width and height arguments), I figured it would be better to start a
 discussion to decide what a new approach would require.

 I'd like to propose two possible solutions.

 The first one is relatively-simple and is more of a fix than a solution.
 The core issue seems to be that it is difficult to recognize specific
 Thickboxes from the main page looking into the Thickbox. Thus, the code
 simply targets everything. I suggest reversing the way this is structured.

 Rather than housing the resize code outside of the Thickbox and modifying
 all of them, the resize code could be loaded by the wp-admin/media-
 upload.php page (the page loaded by the iFrame inside the media uploader's
 Thickbox) and use outward-looking checks to discover information about the
 window size. This will cause the code to be reliably run only on media
 uploader Thickboxes and removes any possible interference with other uses
 of Thickboxes (even if other non-media uploader Thickboxes exist on the
 same page).

 My biggest concern with this type of approach is that it isn't really
 elegant. If WordPress core needs a solution to finely control how
 Thickboxes size and auto-resize themselves, it stands to reason that other
 code probably has need to do this as well.

 My second suggestion is to improve the Thickbox code to handle a more
 robust set of query arguments. For example a link such as the following
 would emulate the current behavior of the the media-upload resize code.

 {{{
 link?TB_iframe=1&width=full&height=full&max-width=720
 }}}

 There would be four attributes that would be the main focus:

 * width
     * full - Expand to fill the full width of the containing window
     * content (default) - The width automatically matches the width of the
 content
     * [int value] - A specific pixel width
 * height
     * full - Expand to fill the full height of the containing window
     * content (default) - The height automatically matches the height of
 the content
     * [int value] - A specific pixel height
 * max-width
     * full (default) - The width will be limited to that of the "full"
 width
     * [int value] - The width will be limited to a specific pixel width
 * max-height
     * full (default) - The height will be limited to that of the "full"
 height
     * [int value] - The height will be limited to a specific pixel height

 All of the calculations will naturally also handle compensating sizes for
 necessary margin and padding sizes as well as compensating for the
 presence of the admin bar.

 The code will also automatically handle resize events to ensure 1) that
 the current attribute options are respected and 2) that the Thickbox
 doesn't overflow the window.

 With a setup like this, Thickboxes become much more useful to the average
 developer and an annoying code conflict source is removed.

 Thoughts?

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/17249#comment:1>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software