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

LibWeb+WebContent: Move scrollbar painting into WebContent #24554

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jamierocks
Copy link
Contributor

See LadybirdBrowser/ladybird#21 for details :^)

This is a hack needed to preserve current behaviour after making set
viewport_rect() being not async in upcoming changes.

For example both handle_mousedown and handle_mouseup should use the same
viewport scroll offset even though handle_mousedown runs focusing steps
that might cause scrolling to focused element:
- handle_mousedown({ 0, 0 })
  - run_focusing_steps()
  - set_focused_element()
  - scroll_into_viewport() changes viewport scroll offset
- handle_mouseup({ 0, 0 })

(cherry picked from commit 50920b05953a6bc2aacb07d291d503052caadf15)
(cherry picked from commit eb909118bfe9d939a1109823b6e0b8736abab138)
The main intention of this change is to have a consistent look and
behavior across all scrollbars, including elements with
`overflow: scroll` and `overflow: auto`, iframes, and a page.

Before:
- Page's scrollbar is painted by Browser (Qt/AppKit) using the
  corresponding UI framework style,
- Both WebContent and Browser know the scroll position offset.
- WebContent uses did_request_scroll_to() IPC call to send updates.
- Browser uses set_viewport_rect() to send updates.

After:
- Page's scrollbar is painted on WebContent side using the same style as
  currently used for elements with `overflow: scroll` and
  `overflow: auto`. A nice side effects: scrollbars are now painted for
  iframes, and page's scrollbar respects scrollbar-width CSS property.
- Only WebContent knows scroll position offset.
- did_request_scroll_to() is no longer used.
- set_viewport_rect() is changed to set_viewport_size().

(cherry picked from commit 5285e22f2aa09152365179865f135e7bc5d254a5)
No longer used after moving scrollbar painting into WebContent.

(cherry picked from commit cc3d95a356ea3769d325c2f7bb73947cb4ba7baa)
No longer used after moving scrollbar painting into WebContent.

(cherry picked from commit 94eacf6da736f957a5fc22faa552341165aaf1ca)
(cherry picked from commit 881e97084625184543b93cee235cb0b96ee055ae)
@kalenikaliaksandr
Copy link
Contributor

Without making changes in SerenityOS Browser you will get duplicated scrollbar (one painted in WebContent and another in Browser) and probably some other issues.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 9, 2024
@jamierocks
Copy link
Contributor Author

Hmm, silly me. I'll work on this later / tomorrow :)

@jamierocks jamierocks marked this pull request as draft June 9, 2024 20:00
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants