Opened 3 years ago
Closed 3 years ago
#42052 closed defect (bug) (fixed)
Customize Themes: Safari bugs in rendering themes browser header, theme details, and panel contents
Reported by: | afercia | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.9 | Priority: | high |
Severity: | critical | Version: | 4.9 |
Component: | Customize | Keywords: | has-screenshots needs-patch |
Focuses: | Cc: |
Description
Screenshot:
the header with search (and filters) is not visible in Safari 11 for me.
In my case, the media query screen and (max-height:540px), screen and (max-width:1018px)
kicks in when I open the Safari Web Inspector (keeping the inspector panel to a certain height) and switches the header to position relative so, a bit ironically, it gets visible again when trying to debug. But when the header has position fixed, it is not visible.
Suggestion: investigate overflow-y: scroll;
set on ul.customize-themes-full-container
See #37661.
Attachments (7)
Change History (47)
#2
@
3 years ago
- Owner celloexpressions deleted
I don't have access to Safari, so I can't test fixes here and am probably not the best person to work on a fix. @afercia does bumping the z-index on .customize-preview-header.themes-filter-bar
help?
#3
@
3 years ago
@celloexpressions z-index
is the first thing I've tried but that didn't help. Haven't fully figured out why this happens, not exluding a Safari bug.
This ticket was mentioned in Slack in #core by afercia. View the logs.
3 years ago
#8
@
3 years ago
42052.diff looks okay to me - I don't see any regressions in Chrome although I can't verify if it fixes the Safari issue. In addition to the basic behavior, be sure to test toggling the feature filters and at window/screen sizes where the filter bar is fixed and where it's not sticky - the margin and padding here are related to those distinct cases.
#9
follow-up:
↓ 10
@
3 years ago
@benoitchantre note that it isn't necessary to patch the RTL files - those are done automatically as part of the build process. Preference is also to patch all the way back to src/wp-admin/css/customize-controls.css
, working off of the develop repository. There's more information in the handbook, or you can ask in Slack for additional clarification on how that works.
#10
in reply to:
↑ 9
@
3 years ago
Replying to celloexpressions:
@benoitchantre note that it isn't necessary to patch the RTL files - those are done automatically as part of the build process. Preference is also to patch all the way back to
src/wp-admin/css/customize-controls.css
, working off of the develop repository. There's more information in the handbook, or you can ask in Slack for additional clarification on how that works.
Thanks for the info, I'm starting to contribute and doesn't know everything yet. I think I need to install Vagrant and everything, because at the moment I have no src directory.
Do I need to update the patch or is it ok?
This ticket was mentioned in Slack in #core by westonruter. View the logs.
3 years ago
#12
@
3 years ago
- Owner set to afercia
- Status changed from assigned to reviewing
@benoitchantre you don't need to supply a new patch to remove the RTL changes. Just be aware you can save yourself time and skip modifying the RTL.
You don't need the src
directory as grunt:patch
can apply it either way.
You can read more at https://make.wordpress.org/core/handbook/tutorials/working-with-patches/
@afercia would you review and commit if you confirm it fixes the issue?
This ticket was mentioned in Slack in #core by melchoyce. View the logs.
3 years ago
#14
follow-ups:
↓ 16
↓ 19
@
3 years ago
Tested 42052.diff and in the responsive view the "back" button is not visible in Safari 11
As mentioned in the ticket description, removing the overflow-y: scroll;
set on ul.customize-themes-full-container
seems to fix it. However, I'm not sure if there are side-effects. I think this should be investigated in depth since we're patching it but we don't know exactly _what_ we are patching. In other words, the real reason of this bug hasn't been identified.
#15
@
3 years ago
- Owner afercia deleted
Sorry I don't have time to follow this ticket as owner. Best option would be a person whos; more familiar with the implementation and the CSS used here.
#16
in reply to:
↑ 14
@
3 years ago
Replying to afercia:
As mentioned in the ticket description, removing the
overflow-y: scroll;
set onul.customize-themes-full-container
seems to fix it. However, I'm not sure if there are side-effects. I think this should be investigated in depth since we're patching it but we don't know exactly _what_ we are patching. In other words, the real reason of this bug hasn't been identified.
When overflow-y: scroll;
is removed, the scroll doesn't work anymore. I would like to investigate more, but the situation is a bit complicated now for me (I'm dad for the second time since Monday and have no time to contribute during office hours). I'll see what I can do in the next few days.
#19
in reply to:
↑ 14
@
3 years ago
Replying to afercia:
Tested 42052.diff and in the responsive view the "back" button is not visible in Safari 11
This is now fixed in 42052.4diff (one dot is missing in the filename).
I'll try to understand the root cause.
#21
follow-up:
↓ 27
@
3 years ago
@benoitchantre I committed your fix in [41863] while you're looking into the root cause, but in my testing I found another big problem with Safari and that is when you start typing into the search field, the sections on the left slide out into oblivion: https://youtu.be/6s8qCJy90AY
Would you be able to dig into that as well?
#22
@
3 years ago
when you start typing into the search field, the sections on the left slide out into oblivion: https://youtu.be/6s8qCJy90AY
This happens also in two more scenarios for me:
- when tabbing from the sidebar to the search field: the sidebar content disappears
- when opening a theme details modal and then closing it: the sidebar content disappears
Seems to me .wp-full-overlay-sidebar-content
just scrolls horizontally, notice some of the following panels there do take space even if they use visibility: hidden; height: 0;
they actually have some padding padding: 12px;
. Strangely enough, removing this padding fixes the scrolling but still doesn't identify the root cause.
The scrolling seems to happen just in Safari though, I guess because Safari has a different (buggy? stricter?) overflow implementation. Just googling a bit, there are several references to not expected overflow behaviors with Safari. This doesn't explain why the scrolling happens when typing in the search field though. I guess there's something going on with the translateX
for panels that are translated to the right together with overflow
weirdness in Safari but couldn't identify what triggers the scrolling.
#23
@
3 years ago
About the details modal being clipped in Safari:
The only reference I was able to find that could partially explain the Safari behavior is in the W3C CSS basic box model working draft (outdated but still referenced by the CSS Overflow Module Level 3)
https://www.w3.org/TR/css3-box/#overflow
The computed values of ‘overflow-x’ and ‘overflow-y’ are the same as their specified values, except that some combinations with ‘visible’ are not possible: if one is specified as ‘visible’ and the other is ‘scroll’ or ‘auto’, then ‘visible’ is set to ‘auto’.
This probably opened the doors to different implementations across browsers.
Long story short, 42052.4.diff sets back the themes container overflow-y
to visible
when the details modal is open, making the modal fully visible in Safari:
The only drawback I've noticed, is that the themes list behind the modal will scroll to the top when the modal opens, and scroll back down when the modal closes. Worth noting this already happens in Chrome and Firefox without the patch applied.
#24
@
3 years ago
- Keywords needs-patch added; has-patch removed
@afercia I'm still not seeing 42052.4.diff fix the issue. Here's what I see with the patch applied: https://youtu.be/kTn5j_H1Uss
#26
@
3 years ago
- Priority changed from normal to high
- Summary changed from Customize Themes: themes browser header not visible in Safari 11 to Customize Themes: Safari bugs in rendering themes browser header, theme details, and panel contents
@afercia Sorry, I see you were specifically fixing the one clipping issue.
when opening a theme details modal and then closing it: the sidebar content disappears
Here is a video depicting this issue: https://youtu.be/ehp9ViukViE
We need to figure out why panel's contents are disappearing in Safari.
#27
in reply to:
↑ 21
@
3 years ago
Replying to westonruter:
@benoitchantre I committed your fix in [41863] while you're looking into the root cause, but in my testing I found another big problem with Safari and that is when you start typing into the search field, the sections on the left slide out into oblivion: https://youtu.be/6s8qCJy90AY
Would you be able to dig into that as well?
My free time is now near to 0. I have looked at that a bit yesterday, but not enough to find a solution. Can someone take the relay?
#29
@
3 years ago
After playing around with this for a bit, I believe that the disappearing issue Weston mentioned is a bug in Safari. You can get the element to reappear in the correct location simply by unchecking and rechecking the positioning on the .wp-full-overlay-sidebar-content
element.
#30
@
3 years ago
Tried to debug and see if it is related to the translate
property on the parent and child panes, but there was also no change to the disappearing act.
#31
@
3 years ago
- Severity changed from normal to critical
Yeah, this is still looking really, really broken in Safari: https://cloudup.com/cy6VMq981Jd
Increasing severity.
This ticket was mentioned in Slack in #core by westonruter. View the logs.
3 years ago
#34
@
3 years ago
This seems to be a bug, but in general and from what I have learned from my research is that some browsers break things when we give position:fixed
to an element and its parent has transform
property set.
I didn't find any solution for it anywhere, but please see my patch to see what I have come up with.
This fixes the problem of section disappearing in safari. I have yet to work on the other issues @melchoyce mentioned.
#36
@
3 years ago
And this is the CSS way, which should fix the problem of section disappearing when searching themes or closing modal.
This ticket was mentioned in Slack in #core-customize by sayedwp. View the logs.
3 years ago
#38
@
3 years ago
- Owner set to westonruter
Looks good to me, but could use a second review just in case 👍
Note some changes to z-index are pending in https://github.com/xwp/wordpress-develop/pull/267