Try: Safari flickering fix, take two #32188
Conversation
jasmussen
added
Browser Issues
|
Size Change: +155 B (0%) Total Size: 1.86 MB
ℹ️ View Unchanged
|
|
I haven't used I'm also still unsure if this is a thing we need to fix on our end, or if there was a regression in webkit... this wasn't a problem until recently. |
| @@ -169,6 +169,9 @@ | |||
| box-shadow: 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color); | |||
| border-radius: $radius-block-ui - $border-width; // Border is outset, so subtract the width to achieve correct radius. | |||
|
|
|||
| // This renders the border on the GPU which fixes a Safari flicker issue. | |||
| will-change: transform; | |||
There was a problem hiding this comment.
What transform is this referring to?
There was a problem hiding this comment.
No transform. It's meant to tell the GPU to render this element on the GPU, and I chose this instead of something like transform: translateZ(0); which I would consider even more of a hack.
Definitely — the thing is, this is what appears to be causing the flicker to happen:
Look how the top value changes as you scroll down the page. This value change appears to be almost in sync with the flicker, and perhaps through shotgun debugging, applying the will-change property to the thing that flickered, in this case the focus style, the issue was gone. So to an extent, you could argue it may be the software renderer that is stressed with the amount of nodes on the page. But it still feels appropriate to use this value. Though I suspect |
|
This does seem to fix the Gallery/Media + Text issues, but the more intense flickering caused when there's a fixed background are still present:
Could/should this PR be expanded to help with that? |
|
Good news and bad news. I think I fixed the cover in Safari:
Bad news: will-change affected the focus style in Chrome, causing it to be slightly cropped, like so:
I "fixed" that, by juggling some pixel values around (see 3e2af96):
but it does mean that will-change is affecting the rendering of the things on which it's applied, so we need to test extra carefully focus styles across blocks, in nesting contexts and whatnot. Because I now also applied it to the Cover overlay (to fix the flickering), we also need to test the Cover overlay well. |
|
How does this look in the iframed editors? Does it affect the content in the frame when the block toolbar is outside of the frame? |
|
It's worth noting that in an iframe, the positioning updates are reversed: when the block toolbar is sticky, it's not needed to update the position, but when the toolbar "follows" the block, it is needed to update the position. |
jasmussen
force-pushed
the
try/safari-flicker-fix-alt
branch
from
3e2af96 to
f8b88fb
Compare
Damn good instinct on you. It appears as though the iframe solves this problem entirely. Here are two tabs in safari, the first one the site editor, the second one, the post editor. One flickers, the other does not:
In other words, if we were to roll out the iframe to the post editor, that would also solve the flickering. I suppose the question is: do we want to include this patch in the mean time? |
|
@kjellr Note how the iframe also appears to solve the issue with the Cover image fixed background position. |
|
@jasmussen Nice! I'm wonder why the popover positioning affects the post content at all. What if the scrollable area gets moved to |
|
I took a quick stab at some web-inspecting to see if moving the scroll container to editor-styles-wrapper would do it. Doesn't seem like it, though it was a superficial inspecting:
I also tried, in a fit of shotgut debugging, of removing virtually all dom nodes and all CSS from wp-admin to see if it was bleed. That also didn't fix it. As best I can tell: when when animating an element over another element that uses z-index in Safari, the element under the animated one will flicker, if they are in the same document. As soon as you do it over an iframe, it's fine. This is a guess, of course. |
|
Searching the topic, this random comment stood out:
In our case, the things that are flickering are the focus styles which are rendered on an abs-positioned pseudo element, and the cover block overlay, which is both abs positioned and has an opacity, so it rings true. So the animation of the element above the abs positioned element is causing a re-render, which is the flicker. I agree it's weird that the iframe fixes that, presumably the two still need to be composited. But it seems like it does. |
|
Hm, I'll try it quick in a bit. In my head it should work out because there would be no animation above the element. Let me try it this afternoon quick. |
|
Closing this one in favor of #32575, which is streamlined. |
Description
Better alternative to #31412 which in my testing appears to accomplish the same.
The block toolbar is absolutely positioned. When it "unsticks" and scrolls with a block down the page, a la how
position: sticky;works, due to a refactor, in actualitytopvalues are updated on the block toolbar to emulate this behavior.This specific behavior appears to interfere with some other abs positioned layers, causing them to flicker.
Before:
After:
How has this been tested?
Please test blocks in Safari. Notably Media & Text and the Gallery appear to be affected. You can use this test content:
Add that, then select either block, and scroll down the page so the block toolbar of the selected block "unsticks" and starts scrolling with you down the page. You should not see any flicker.
Note
I think this is a good fix.
will-change: transformis a way to tell the GPU to render the element. In this case it fixes the issue, and in all cases it has a theoretical performance benefit (which in this case would be mostly negligible).But the rule would apply to all browsers, so it would be good to verify it doesn't regress anything in any browser. So please test also Chrome and Firefox. Specifically, the rule change affects the blue focus style that appears when you select a block. Things to look for is width changes, radius changes, z index changes, other cropping. Nothing should be affected, but I have seen some of these behaviors in rare instances. Testing nested blocks would be good.
Remember, you can also test on Gutenberg run, here: http://gutenberg.run/32188
Checklist:
*.native.jsfiles for terms that need renaming or removal).