Make WordPress Core

Opened 14 years ago

Closed 8 years ago

#10840 closed defect (bug) (fixed)

Plugin upgrade sometimes shows a scrollbar

Reported by: xibe's profile xibe Owned by: nacin's profile nacin
Milestone: 3.6 Priority: low
Severity: normal Version: 2.8.4
Component: Upgrade/Install Keywords:
Focuses: ui, administration Cc:

Description

Please see attached screenshot, made in Safari 4.0.3 on OS X.

The iframe only contains the "Plugin reactivated successfully" line.

Increasing the window's height or width does not make it disappear, or even evolve.

Attachments (2)

wp-update-iframe.png (51.6 KB) - added by xibe 14 years ago.
As seen in the wild.
10840.diff (285 bytes) - added by kovshenin 11 years ago.

Download all attachments as: .zip

Change History (23)

@xibe
14 years ago

As seen in the wild.

#1 @dd32
14 years ago

Yes, An iframe is used for that single line of text.

The reasoning is, That the status of the re-activation can only be known on a new page load, and/or if there is a Fatal PHP error. Loading it in an iframe allows for the status to be shown on the page, and if a error occurs, for the plugin not to be activated causing the WordPress install from being inaccessible.

As for the scrollbars, I think they're supposed to be hidden, Or at least, they're not visible in most browsers i've tested with..

It nearly looks like the inner content has a higher min-height than the height of the iframe..

#2 @scribu
14 years ago

  • Component changed from Upgrade/Install to UI
  • Milestone changed from 2.8.5 to 2.9
  • Summary changed from Plugin upgrade sometimes shows an iframe to Plugin upgrade sometimes shows a scrollbar

#3 @ryan
14 years ago

  • Milestone changed from 2.9 to Future Release

#4 @ocean90
13 years ago

  • Keywords needs-patch added

They are visible in Chrome und Safari, the issue is html, body { height: 100% } from global.css.

#6 @sabreuse
11 years ago

  • Component changed from UI to Plugins
  • Keywords ui-focus added

#7 follow-up: @helen
11 years ago

Still a thing?

#8 in reply to: ↑ 7 @sabreuse
11 years ago

Replying to helen:

Still a thing?

Now that you mention it, I'm not sure -- I haven't seen it any time lately, but that how testing a negative on a "sometimes" goes...

#9 @xibe
11 years ago

Initial reporter here. Cannot test again, as I do not have access to an OS X machine anymore.

#10 @kovshenin
11 years ago

Yes it's still a thing. I just reproduced this with latest trunk, Safari 6.0.2 and latest Chrome on OS X 10.8.2. Although it's less ugly since the scroll bars are hidden unless you scroll: http://cl.ly/image/0R0n3q322C0s

To reproduce this you need to update a plugin that is currently active.

@kovshenin
11 years ago

#11 @kovshenin
11 years ago

  • Cc kovshenin added
  • Keywords has-patch needs-testing added; needs-patch removed

Looks like the paragraph top margin is overflowing the body element, causing that extra margin from the top. 10840.diff adds a 1px padding-top to the body in the iframe to prevent that. Tested in Chrome and Safari under OS X, would appreciate some Windows testing too.

Also note that we use the same iframe code in bulk upgrades as well as theme upgrades. See iframe_header and iframe_footer in wp-admin/includes/template.php for more info.

#12 @SergeyBiryukov
11 years ago

  • Milestone changed from Future Release to 3.6

#13 @dmcneillie
11 years ago

The iframe is set to a height of 170px and the p tag has a margin: 1em 0; causing the p of the iframe to exceed the height of the iframe it sits in. The overflow has been set to hidden, but it still generates the scroll bar because the p tag is taller than the iframe.

The p {margin: 1em 0;} is a global style.

I've got a fix on my local machine to line 81 of update.php

iframe_header( ('Plugin Reactivation'), true );

if ( isset($_GETsuccess?) )

echo '<p style="margin:0;">' . ('Plugin reactivated successfully.') . '</p>';

#14 @xibe
11 years ago

Finally seen it in the wild again. Capture with Chrome Version 26.0.1410.43 m: http://i.imgur.com/gIYot5A.png

Yeah, beta 1, I know...

#15 @nacin
11 years ago

Buried in wp-admin.css:

/* Scrollbar fix for bulk upgrade iframe */
body.iframe {
	height: 98%;
}

It appears removing that causes problems even with 10840.diff.

I'm going to toss in 10840.diff because it really can't hurt — and in Chrome on Mac OS X, it fixes the problem for me.

#16 @nacin
11 years ago

In 24631:

Avoid a scrollbar in the iframe used for plugin sandboxing. props kovshenin. see #10840.

#17 @nacin
11 years ago

Despite [24631], moving this to future release to see if we can consolidate or improve on these fixes. If not, someone please eventually move it back to 3.6 and mark it as fixed.

#18 @nacin
11 years ago

  • Milestone changed from 3.6 to Future Release

#19 @nacin
10 years ago

  • Component changed from Plugins to Upgrade/Install
  • Focuses administration added

#20 @chriscct7
8 years ago

  • Keywords has-patch needs-testing removed
  • Milestone changed from Future Release to 3.6
  • Owner set to nacin
  • Status changed from new to assigned

#21 @chriscct7
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.