serenity Ladybird: Include scroll bar width/height in viewport rect

Apr 24 '24 18:04 jamierocks

Can you explain what this fixes? Won't this make LibWeb try to paint underneath the scrollbars?

Apr 24 '24 21:04 trflynn89

Practically, I've observed this resolve issues where background-size: cover is used (previously I've observed blank 'bars' next to the background image).

The viewport rect is also used for window.innerWidth and window.innerHeight, the specification says:

The innerWidth attribute must return the viewport width including the size of a rendered scroll bar (if any), or zero if there is no viewport.

The innerHeight attribute must return the viewport height including the size of a rendered scroll bar (if any), or zero if there is no viewport.

Apr 24 '24 22:04 jamierocks

Hmm yeah the innerWidth / innerHeight thing certainly seems like a bug, but I don't know this is the right fix. For example, with this patch, part of https://github.com/SerenityOS/serenity does get painted underneath scrollbar, and I have no way to scroll rightward enough to see the part that is hidden:

Before this patch: before

With this patch: after

Apr 25 '24 13:04 trflynn89

It certainly seems wrong that the scroll bar would cover the screen contents. Looking at other browsers, most render the scroll bar as a floating bar - so get away with rendering over the page's contents, but those that have a full scrollbar (like Ladybird) do render as you would expect.

Browser	Image	Notes
Firefox		Floating scroll bar is visible, but background isn't until hover-over (shown).
Gnome Web		Floating scroll bar is visible, expands on hover-over (shown).
Chromium		Scroll bar is always visible, never covers content.

I think the solution is to set 2 viewport rects - one including the scroll bars, and one without.

I'll work on making a small reproduction of the background-size: cover issue I had, and then look at doing this differently.

Apr 25 '24 17:04 jamierocks

Okay - so it turns out the issue I was having was to do with background-position: centre and background-size: cover. It also seems that even with the proposed change, if you tweak the window size you still see white bars. I think some other issue is at play there.

<style>
    body {
        margin: 0;
        padding: 0;

        height: 150vh;
    }

    #test {
        width: 100%;
        height: 100vh;

        background: url(https://images.placeholders.dev/?width=2200&height=1311&text=WHF!);
        background-size: cover !important;
        background-position: center !important;
    }
</style>
<body>
    <div id="test"></div>
</body>

Gnome Web	Ladybird (Without change)

	Notice the white bar that appears to the left-hand side.

Okay, tangent aside - I'll look into that again another time. Time to start looking into IPC.

Apr 25 '24 18:04 jamierocks

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

May 22 '24 00:05 stale[bot]