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

Scrollbar does not pick up changed styles until hovered #12312

Conversation

obyknovenius
Copy link
Contributor

@obyknovenius obyknovenius commented Apr 3, 2023

dfd97e6

Scrollbar does not pick up changed styles until hovered
https://bugs.webkit.org/show_bug.cgi?id=113727

Reviewed by Simon Fraser.

Resolve the pseudo-element `::-webkit-scrollbar` in the same way as it
is done with other pseudo-elements that can be applied to any element.

* Source/WebCore/style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveElement):
(WebCore::Style::TreeResolver::resolvePseudoElement):
* LayoutTests/fast/css/scrollbar-dynamic-style-change-expected.html: Rebaselined
* LayoutTests/fast/css/scrollbar-dynamic-style-change.html: Ditto

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

532906f

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 βœ… πŸ§ͺ mac-wk2-stress
❌ πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@obyknovenius obyknovenius self-assigned this Apr 3, 2023
@obyknovenius obyknovenius added the CSS Cascading Style Sheets implementation label Apr 3, 2023
@obyknovenius obyknovenius added the request-merge-queue Request a pull request to be added to merge-queue once ready label Apr 3, 2023
@pxlcoder
Copy link
Member

pxlcoder commented Apr 3, 2023

Can we add a test for this?

@pxlcoder pxlcoder requested review from smfr and anttijk April 3, 2023 18:04
@obyknovenius
Copy link
Contributor Author

We kinda have a test about this, it's fast/css/scrollbar-dynamic-style-change.html. But it passes on Mac by some happy accident. I couldn't figure out why. Something else is happening that causes style update.

@obyknovenius
Copy link
Contributor Author

@pxlcoder @smfr @anttijk I've updated fast/css/scrollbar-dynamic-style-change.html to negate the happy accident. The test should now fail on Mac too without this patch.

@Ahmad-S792
Copy link
Contributor

Ahmad-S792 commented May 22, 2023

@pxlcoder @smfr @anttijk I've updated fast/css/scrollbar-dynamic-style-change.html to negate the happy accident. The test should now fail on Mac too without this patch.

Can you update 'commit message' to include changed files (test files)? Like below:

* LayoutTests/fast/css/scrollbar-dynamic-style-change-expected.html: Rebaselined
* LayoutTests/fast/css/scrollbar-dynamic-style-change.html: Ditto

@obyknovenius obyknovenius force-pushed the fix/css/scrollbar-dynamic-style-change branch from e656299 to 532906f Compare May 22, 2023 15:34
@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed request-merge-queue Request a pull request to be added to merge-queue once ready labels May 22, 2023
@Ahmad-S792 Ahmad-S792 added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels May 22, 2023
@webkit-commit-queue webkit-commit-queue force-pushed the fix/css/scrollbar-dynamic-style-change branch from 532906f to fbd9526 Compare May 22, 2023 20:09
https://bugs.webkit.org/show_bug.cgi?id=113727

Reviewed by Simon Fraser.

Resolve the pseudo-element `::-webkit-scrollbar` in the same way as it
is done with other pseudo-elements that can be applied to any element.

* Source/WebCore/style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveElement):
(WebCore::Style::TreeResolver::resolvePseudoElement):
* LayoutTests/fast/css/scrollbar-dynamic-style-change-expected.html: Rebaselined
* LayoutTests/fast/css/scrollbar-dynamic-style-change.html: Ditto

Canonical link: https://commits.webkit.org/264356@main
@webkit-commit-queue webkit-commit-queue force-pushed the fix/css/scrollbar-dynamic-style-change branch from fbd9526 to dfd97e6 Compare May 22, 2023 20:11
@webkit-commit-queue
Copy link
Collaborator

Committed 264356@main (dfd97e6): https://commits.webkit.org/264356@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 22, 2023
@webkit-commit-queue webkit-commit-queue merged commit dfd97e6 into WebKit:main May 22, 2023
@obyknovenius obyknovenius deleted the fix/css/scrollbar-dynamic-style-change branch May 23, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Cascading Style Sheets implementation
Projects
None yet
7 participants