WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#17473 closed defect (bug) (fixed)

Add JS detection to wp_iframe output

Reported by: jacobwg Owned by: azaozz
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.2
Component: General Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Currently any page in the admin area includes the necessary script to add the js and no-js classes to the body (from admin-header.php), but this code is missing from the output of the wp_iframe function.

In particular, the upload media popup (/wp-admin/media-upload.php) makes use of hide-if-no-js classes, and without the detection script in place in wp_iframe, those classes will not take effect.

Attachments (2)

17473.diff (728 bytes) - added by jacobwg 10 years ago.
Adds body class and JS code necessary for the hide-if-js and hide-if-no-js classes to function
17473-2.diff (544 bytes) - added by jacobwg 10 years ago.
Adds only the 'js' class - no JS detection

Download all attachments as: .zip

Change History (12)

@jacobwg
10 years ago

Adds body class and JS code necessary for the hide-if-js and hide-if-no-js classes to function

#1 @jacobwg
10 years ago

  • Keywords has-patch added

#2 @greuben
10 years ago

  • Keywords 2nd-opinion added

In particular, the upload media popup

These popups only work if js is enabled. So adding them makes no sense

#3 @jacobwg
10 years ago

The classes exist and should function - in particular the hide-if-js class should function as JS is enabled.

Yes, the popups only work if js is enabled, but the wp_iframe function is not JS specific.

#4 @scribu
10 years ago

The classes exist and should function - in particular the hide-if-js class should function as JS is enabled.

Yes, but your patch doesn't address that, or rather it could do it more directly, by just adding the 'js' class, if that's what's needed.

#5 @jacobwg
10 years ago

Should I just add the js class, then, or is there ever a case where we'll use an iframe for something that might work without JS enabled?

@jacobwg
10 years ago

Adds only the 'js' class - no JS detection

#6 @Dd32
10 years ago

The first patch looks like the way to go, allows the maximum compatibility and reuse of code between iframes and the normal admin pages

#7 @azaozz
10 years ago

Was thinking to add the second patch for the reason @greuben points out, but @Dd32 makes a good point too. Currently we don't use this iframe outside the JS dialog but we may use it in the future.

#8 @dd32
10 years ago

I was mainly thinking of plugins injecting HTML into the iframes, I'd expect plugins to re-use html between iframe and non-iframe pages.. having the js css classes avilable would be of great benefit for non-js support in those cases.

#9 @azaozz
10 years ago

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

In [17971]:

Add JS detection to wp_iframe output, remove "display:none" from the Flash uploader button, props jacobwg, fixes #17473

#10 @ocean90
10 years ago

  • Milestone changed from Awaiting Review to 3.2
Note: See TracTickets for help on using tickets.