WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#18796 closed defect (bug) (fixed)

thickbox.css needs display:none

Reported by: OoroonBai Owned by: azaozz
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: External Libraries Keywords: thickbox has-patch
Focuses: 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 3 years ago.
18796.overlap.png (7.7 KB) - added by SergeyBiryukov 3 years ago.
18796.2.patch (2.3 KB) - added by SergeyBiryukov 3 years ago.
thickbox.18796.patch (1.9 KB) - added by OoroonBai 3 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 TobiasBg3 years 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 SergeyBiryukov3 years ago

  • Milestone changed from Awaiting Review to 3.3

comment:3 duck_3 years ago

Also noticed in #18815

SergeyBiryukov3 years ago

SergeyBiryukov3 years ago

SergeyBiryukov3 years ago

comment:4 follow-up: SergeyBiryukov3 years 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.

OoroonBai3 years ago

comment:5 in reply to: ↑ 4 OoroonBai3 years 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 SergeyBiryukov3 years 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 azaozz3 years 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 SergeyBiryukov3 years ago

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

comment:9 OoroonBai3 years 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 SergeyBiryukov3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:11 azaozz3 years 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.