Opened 9 years ago
Closed 9 years ago
#36276 closed defect (bug) (fixed)
Refreshed Customizer preview iframe shows before initialized
Reported by: | westonruter | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.5 | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Customize | Keywords: | has-patch commit |
Focuses: | Cc: |
Description (last modified by )
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: https://cloudup.com/c1V95W-wXkJ
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: https://core.trac.wordpress.org/ticket/31195#comment:79
This fixes the issue for me:
-
src/wp-admin/css/customize-controls.css
p.customize-section-description { 613 613 height: 100%; 614 614 position: absolute; 615 615 } 616 #customize-preview iframe + iframe { 617 visibility: hidden; 618 } 616 619 617 620 .wp-full-overlay-sidebar { 618 621 background: #eee;
Attachments (1)
Change History (10)
This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.
9 years ago
#4
@
9 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
@
9 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.
9 years ago
#7
@
9 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
@
9 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.
Are there any issues with using
visibility:hidden
on aniframe
? I have some feeling that it may not work reliably across browsers.