Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#36276 closed defect (bug) (fixed)

Refreshed Customizer preview iframe shows before initialized

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.6
Component: Customize Keywords: has-patch commit
Focuses: Cc:

Description (last modified by westonruter)

I've noticed that when the Customizer preview is refreshed while the user has scrolled down from the top, the refreshed iframe will momentarily be displayed being scrolled to the top before scroll back to the scroll position at the time the refresh was requested. This issue was not present in 4.4. Here's a depiction of problem:

I've traced the issue down the regression to [36532] with the introduction of the device preview functionality (#31195). Specifically, it seems this change is causing the problem:

  #customize-preview iframe {
        width: 100%;
        height: 100%;
+       position: absolute;

When the preview is being refreshed, a second iframe is inserted as the next sibling of the existing iframe. Because they now have position:absolute, the second iframe now is getting displayed on top of the first iframe. The second iframe will initially appear without the proper scroll position: only once the iframe finishes loading will its scroll position be set, and the old iframe is removed from the document.

Here is why position:absolute was introduced:

This fixes the issue for me:

  • src/wp-admin/css/customize-controls.css

    p.customize-section-description { 
    613613        height: 100%;
    614614        position: absolute;
     616#customize-preview iframe + iframe {
     617        visibility: hidden;
    617620.wp-full-overlay-sidebar {
    618621        background: #eee;

Attachments (1)

36276.0.diff (417 bytes) - added by westonruter 8 years ago.

Download all attachments as: .zip

Change History (10)

8 years ago

#1 @westonruter
8 years ago

  • Description modified (diff)

Are there any issues with using visibility:hidden on an iframe? I have some feeling that it may not work reliably across browsers.

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.

8 years ago

#3 @westonruter
8 years ago

  • Keywords needs-testing added

#4 @celloexpressions
8 years ago

This sounds like it should work in theory. My primary concern would be whether this causes any accessibility issues, although I'm not sure how accessible the preview refreshing (or partial refreshing) is right now anyway.

#5 @obenland
8 years ago

@celloexpressions What type of a11y issue in specific do you anticipate?

Patch looks sane to me otherwise.

This ticket was mentioned in Slack in #accessibility by mike. View the logs.

8 years ago

#7 @celloexpressions
8 years ago

Per @afercia on slack, this probably won't make the accessibility issues with the preview worse so we can proceed with the patch I think. There's definitely accessibility work to do there in the future, though.

#8 @westonruter
8 years ago

  • Keywords commit added; needs-testing removed
  • Owner set to westonruter
  • Status changed from new to accepted

@celloexpressions @afercia The whole point of the patch is to ensure the secondary iframe is not accessible. Only the first iframe should be displayed because it is the one that is fully loaded. Once the secondary iframe is fully loaded, then the primary (first) iframe gets destroyed and removed, so that the secondary iframe becomes the primary one. And at this point it will be visible.

It is also important to note that once transactions (#30937) are implemented, there should only ever be one iframe that is just reloaded, so this issue will be obsolete.

#9 @westonruter
8 years ago

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

In 37060:

Customize: Prevent preview iframe from showing until fully loaded.

Fixes issue where the Customizer preview window can appear to momentarily bounce to the top when being refreshed. Regression from [36532].

See #31195.
Fixes #36276.

Note: See TracTickets for help on using tickets.