Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect display of container list #1485

Open
hacketiwack opened this issue Nov 18, 2023 · 12 comments
Open

Incorrect display of container list #1485

hacketiwack opened this issue Nov 18, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@hacketiwack
Copy link

Cockpit version: 304-1
Cockpit-podman version: 80-1
Podman version: 4.7.2
OS: Arch Linux
Browser: Safari 17.1

Description
The display of the list of containers presents broken borders around each elements of the list. See below.
Broken borders

Steps to reproduce

  1. Open Cockpit podman in Safari
  2. The list of items is not presented correctly

Suggestion
It could be wise to simplify the display of this list and just use on separation line between each item and align what is done in other lists: e.g. Network, Virtual Machines, etc.

@hacketiwack hacketiwack added the bug Something isn't working label Nov 18, 2023
@jelly
Copy link
Member

jelly commented Nov 20, 2023

I have no apple hardware but can reproduce this using ephiphany. Dropping the font-size from the container name makes the row underline show as expected.

@jelly
Copy link
Member

jelly commented Nov 20, 2023

Looking further the border color is on pf-v5-c-table__tr while it looks like it is on the from the screenshot. Webkit bug?
image

Once expanded it is also broken. I changed the border color here for debugging.

image

@garrett
Copy link
Member

garrett commented Nov 20, 2023

(BTW: IIRC, stuff like this is one of the reasons we dropped custom expandable tables in Cockpit-Machines and elsewhere.)

@hacketiwack
Copy link
Author

@garrett BTW, on Cockpit Machines, this is not much better (see below).
I will open a ticket for this as well.

Cockpit-Machines-List-Bug

@jelly, I know this could be a stretch for you. However, you can create a macOS VM if needed.

@jelly
Copy link
Member

jelly commented Nov 21, 2023

@garrett BTW, on Cockpit Machines, this is not much better (see below). I will open a ticket for this as well.
Cockpit-Machines-List-Bug

@jelly, I know this could be a stretch for you. However, you can create a macOS VM if needed.

I can reproduce it fine with epiphany.

@garrett
Copy link
Member

garrett commented Nov 21, 2023

@garrett BTW, on Cockpit Machines, this is not much better (see below). I will open a ticket for this as well.

Oh! Thanks!

Since it's in both of these places, I wonder if this is some kind of regression in PatternFly. I know this all did work fine in WebKit before, and nothing should've changed dramatically in both places since (except for PF).

(We did want to have visual regression tests in WebKit, but it's apparently a hassle or impossible to set up. 😢 Usually, it doesn't matter, as browser engines often are close enough to the same these days... but for stuff like this, it's invaluable.)

Thanks again for reporting this!

I'll probably look at this later today, if @jelly doesn't figure it out first.

@garrett
Copy link
Member

garrett commented Nov 21, 2023

I can't recreate this locally.

Latest Cockpit-Podman from main, GNOME Web 45.1, WebKitGTK 2.42.1,

image

I've also tried with GNOME Web Canary 45.0-54-gc80edb25e, WebKitGTK 2.43.1 (270991@main), which is even more aligned with Safari 17, and it's also fine here.

image

@garrett
Copy link
Member

garrett commented Nov 21, 2023

AHA! If I zoom fonts, I get rendering artifacts:

image

And then, if I reload the page after that, I see something similar:

image

This happens in both stable GNOME Web and Canary.

I don't see the broken border on expanding, but I think if we can figure out a fix for this in general, then it'll probably solve that too.

Now, the question is if this is a bug in Cockpit-Podman (and Cockpit-Machines), in Cockpit itself, in PatternFly, or even in WebKit? (We have seen WebKit-specific bugs that were actually a bug in WebKit over the years, both with how it does lower level stuff like TLS as well as rendering issues too.) And then the next question is if we can figure out a fix or a workaround (depending on where the bug is)?

@garrett
Copy link
Member

garrett commented Nov 21, 2023

So this problem seems to arise from WebKit doing the wrong thing with vertical-align: baseline, and it seems to be a bug in WebKit that's triggered in PatternFly, where they set the baseline alignment in what gets output to this CSS selector: .pf-v5-c-table tbody:where(.pf-v5-c-table__tbody) > tr:where(.pf-v5-c-table__tr) > *

A workaround is to either change the alignment to something like vertical-align: top here, or to set the alignment to baseline and then use vertical-align: -webkit-baseline-middle (which would first set the right thing and then set it to something special for WebKit with a proprietary WebKit specific value... which other browsers will probably ignore).

This doesn't seem to show up on the default text zoom level. It seems (at least here), to only show up when zooming into the page and then reloading the page. As browsers remember the zoom level per site, it will then default to showing this bug if you previously zoomed.

For now, the options are to reset your zoom or ignore the issue while we file this issue upstream and make local workarounds to make WebKit (that is: Safari on macOS, iOS, iPadOS and also GNOME Web) not trigger this problem.

@garrett
Copy link
Member

garrett commented Nov 21, 2023

PatternFly itself triggers this issue in WebKit. Zoom the page (ctrl and +, or I think command and + on a Mac?) and reload on this page: https://www.patternfly.org/components/table/#actions
image

It's important to look at a table that has mixed content. If it's a simple table with just text, then it will align as expected. But if it has a button or something else in another cell, then the vertical alignment to baseline does not work in WebKit when zoomed.

@garrett
Copy link
Member

garrett commented Nov 23, 2023

This is a problem on HiDPI Linux as well, with WebKit.

Here's GNOME Web:

Screenshot from 2023-11-23 17-14-50

And Cockpit Client:

Screenshot from 2023-11-23 17-17-52

Upstream WebKit issue is @ https://bugs.webkit.org/show_bug.cgi?id=243167

@garrett
Copy link
Member

garrett commented Nov 23, 2023

I'll look at this further next week, if @jelly doesn't find a local work-around (possibly what I mentioned above, but we'd need to test it) in the meantime.

Also: Note how the alignment is just wrong for the caret expanders as well.

I'm guessing we either want the WebKit hack (which probably affects Blink as well) or we set align to top in our specific usage anyway (which may work for us here), based on what I see for the content. We'll want to figure out a local workaround for every table across Cockpit, including Machines (where it is confirmed to be a problem).

It's fine in simpler tables, thankfully. But it'll be trickier to have a good enough generic solution (as a workaround in our PF workarounds file and upstream in PF).

I don't think we want to wait some undetermined time for Apple, Igalia, or whomever to fix WebKit. We should find a workaround before that (and hope for the best for a proper fix).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants