WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 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 3 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 3 years ago.
Adds only the 'js' class - no JS detection

Download all attachments as: .zip

Change History (12)

jacobwg3 years ago

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

comment:1 jacobwg3 years ago

  • Keywords has-patch added

comment:2 greuben3 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

comment:3 jacobwg3 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.

comment:4 scribu3 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.

comment:5 jacobwg3 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?

jacobwg3 years ago

Adds only the 'js' class - no JS detection

comment:6 Dd323 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

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

comment:8 dd323 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.

comment:9 azaozz3 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

comment:10 ocean903 years ago

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