WordPress.org

Make WordPress Core

#18796 closed defect (bug) (fixed)

thickbox.css needs display:none

Reported by: OoroonBai Owned by: azaozz
Priority: normal Milestone: 3.3
Component: External Libraries Version: 3.3
Severity: normal Keywords: thickbox has-patch
Cc:

Description

In thickbox.css display:none was replaced by visibility:hidden (line 52). The thickbox will not be displayed if there is no property 'display', because thickbox checks the property 'display' (line 208 in thickbox.js).
If you use e.g. tb_show in a javascript, it will not show the thickbox.

Attachments (4)

18796.patch (850 bytes) - added by SergeyBiryukov 21 months ago.
18796.overlap.png (7.7 KB) - added by SergeyBiryukov 21 months ago.
18796.2.patch (2.3 KB) - added by SergeyBiryukov 21 months ago.
thickbox.18796.patch (1.9 KB) - added by OoroonBai 21 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 TobiasBg21 months ago

I can confirm this. Have been wondering why my plugin that calls tb_show() through its own call does not work with trunk, but with 3.2.1.
Relevant change seems to have been introduced in [18482].

comment:2 SergeyBiryukov21 months ago

  • Milestone changed from Awaiting Review to 3.3

comment:3 duck_21 months ago

Also noticed in #18815

SergeyBiryukov21 months ago

SergeyBiryukov21 months ago

comment:4 follow-up: SergeyBiryukov21 months ago

  • Keywords has-patch added

18796.patch restores display: none.

But then I've noticed there's a small delay before #TB_window fully opens, leading to overlapping for some part of a second (see the screenshot).

18796.2.patch switches the remaining display references to visibility. Looks like the delay is not happening in that case.

OoroonBai21 months ago

comment:5 in reply to: ↑ 4 OoroonBai21 months ago

Replying to SergeyBiryukov:

But then I've noticed there's a small delay before #TB_window fully opens, leading to overlapping for some part of a second (see the screenshot).

18796.2.patch switches the remaining display references to visibility. Looks like the delay is not happening in that case.

Do you try '.show()' instead of 'viibility' or 'display'? Maybe this fix your problem with overlapping.

thickbox.18796.patch restore 'display: none' and replace '.css('display', 'block')' with '.show()'.

comment:6 SergeyBiryukov21 months ago

Still see the overlapping in Firefox 7 with thickbox.18796.patch. According to jQuery docs, .show() just sets display: block internally.

Also, there's a typo in thickbox.css, should be display: none.

comment:7 azaozz21 months ago

This was changed because the uploader had problems initializing in a hidden iframe in some browsers. Seems 18796.2.patch is the proper solution here and the overlapping can probably be solved with css.

comment:8 SergeyBiryukov21 months ago

With 18796.2.patch, there's no overlapping, so should be fine as is.

comment:9 OoroonBai21 months ago

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

I tested 18796.2.patch with some of my plugins. All work fine. I think we can close the ticket.

comment:10 SergeyBiryukov21 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Tickets are closed when a commit is made to the WordPress trunk.

comment:11 azaozz21 months ago

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

In [18848]:

Fix thickbox (again!), props SergeyBiryukov, fixes #18796

Note: See TracTickets for help on using tickets.