Skip to content

Improve resolving of scrollbar-gutter style #15666

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.

Sign up for GitHub

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

Jump to bottom
Merged
webkit-commit-queue merged 1 commit into from
Jul 8, 2023
Merged

Improve resolving of scrollbar-gutter style #15666

webkit-commit-queue merged 1 commit into from
Jul 8, 2023

Conversation

@lukewarlow
Copy link
Member

@lukewarlow lukewarlow commented Jul 8, 2023 •
edited by webkit-early-warning-system
Loading

7636238

Improve resolving of scrollbar-gutter style
https://bugs.webkit.org/show_bug.cgi?id=259014

Reviewed by Darin Adler.

Move the resolution of scrollbar-gutter styles to its own function and implement for RenderLayerScrollableArea and LocalFrameView.
This will allow implementation of scrollbar-gutter on viewports.

This patch has no behaviour changes.

* Source/WebCore/page/LocalFrameView.cpp:
(WebCore::LocalFrameView::scrollbarGutterStyle const):
* Source/WebCore/page/LocalFrameView.h:
* Source/WebCore/platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::scrollbarGutterStyle const):
* Source/WebCore/platform/ScrollableArea.h:
* Source/WebCore/rendering/RenderLayerScrollableArea.cpp:
(WebCore::RenderLayerScrollableArea::verticalScrollbarWidth const):
(WebCore::RenderLayerScrollableArea::horizontalScrollbarHeight const):
(WebCore::RenderLayerScrollableArea::scrollbarGutterStyle const):
* Source/WebCore/rendering/RenderLayerScrollableArea.h:

Canonical link: https://commits.webkit.org/265882@main

0cd1553

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@lukewarlow lukewarlow requested a review from cdumez as a code owner July 8, 2023 17:04
@lukewarlow lukewarlow self-assigned this Jul 8, 2023
@lukewarlow lukewarlow added the CSS Cascading Style Sheets implementation label Jul 8, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 8, 2023 •
edited
Loading

EWS run on previous version of this PR (hash addc1f9)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@lukewarlow lukewarlow requested review from darinadler, nt1m and smfr July 8, 2023 17:15
darinadler
darinadler approved these changes Jul 8, 2023
View reviewed changes
Source/WebCore/page/LocalFrameView.cpp Outdated
Copy link
Member

@darinadler darinadler Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The renderView check is surprising enough that it needs a comment I think.

Copy link
Member Author

@lukewarlow lukewarlow Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be honest I don't actually know what that checks for. This code is a copy of scrollbarWidthStyle which was a copy of verticalOverscrollBehavior.

Copy link
Member Author

@lukewarlow lukewarlow Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will compile locally without it for both gutter and width and see if it breaks anything

Copy link
Member Author

@lukewarlow lukewarlow Jul 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem to be important will remove it. Will also make a follow up to see if its needed for scrollbar width (just tested and it doesn't seem to be) or the overscrollBehavior functions

Copy link
Member Author

@lukewarlow lukewarlow Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://bugs.webkit.org/show_bug.cgi?id=259018

@lukewarlow lukewarlow force-pushed the scrollbar-gutter-viewport-1 branch from addc1f9 to a2c84d4 Compare July 8, 2023 20:17
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 8, 2023 •
edited
Loading

EWS run on previous version of this PR (hash a2c84d4)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

lukewarlow
lukewarlow commented Jul 8, 2023
View reviewed changes
@lukewarlow lukewarlow requested a review from darinadler July 8, 2023 20:20
@lukewarlow lukewarlow force-pushed the scrollbar-gutter-viewport-1 branch from a2c84d4 to 0cd1553 Compare July 8, 2023 21:04
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 8, 2023 •
edited
Loading

EWS run on current version of this PR (hash 0cd1553)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@lukewarlow lukewarlow added the request-merge-queue Request a pull request to be added to merge-queue once ready label Jul 8, 2023
@Ahmad-S792 Ahmad-S792 added merge-queue Applied to send a pull request to merge-queue and removed request-merge-queue Request a pull request to be added to merge-queue once ready labels Jul 8, 2023
@lukewarlow
Improve resolving of scrollbar-gutter style
7636238 
https://bugs.webkit.org/show_bug.cgi?id=259014

Reviewed by Darin Adler.

Move the resolution of scrollbar-gutter styles to its own function and implement for RenderLayerScrollableArea and LocalFrameView.
This will allow implementation of scrollbar-gutter on viewports.

This patch has no behaviour changes.

* Source/WebCore/page/LocalFrameView.cpp:
(WebCore::LocalFrameView::scrollbarGutterStyle const):
* Source/WebCore/page/LocalFrameView.h:
* Source/WebCore/platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::scrollbarGutterStyle const):
* Source/WebCore/platform/ScrollableArea.h:
* Source/WebCore/rendering/RenderLayerScrollableArea.cpp:
(WebCore::RenderLayerScrollableArea::verticalScrollbarWidth const):
(WebCore::RenderLayerScrollableArea::horizontalScrollbarHeight const):
(WebCore::RenderLayerScrollableArea::scrollbarGutterStyle const):
* Source/WebCore/rendering/RenderLayerScrollableArea.h:

Canonical link: https://commits.webkit.org/265882@main
@webkit-commit-queue webkit-commit-queue force-pushed the scrollbar-gutter-viewport-1 branch from 0cd1553 to 7636238 Compare July 8, 2023 23:37
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Jul 8, 2023

Committed 265882@main (7636238): https://commits.webkit.org/265882@main

Reviewed commits have been landed. Closing PR #15666 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 7636238 into WebKit:main Jul 8, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@darinadler darinadler darinadler approved these changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@smfr smfr Awaiting requested review from smfr

@nt1m nt1m Awaiting requested review from nt1m

Assignees

@lukewarlow lukewarlow

Labels

CSS Cascading Style Sheets implementation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lukewarlow @webkit-early-warning-system @webkit-commit-queue @darinadler @Ahmad-S792