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

Change unclear Color::isValid() to explicit isCurrentColor(). #4528

Conversation

mdubet
Copy link
Contributor

@mdubet mdubet commented Sep 20, 2022

0b28244

Change unclear Color::isValid() to explicit isCurrentColor().
https://bugs.webkit.org/show_bug.cgi?id=245424

Reviewed by Antoine Quint.

Due to the representation of "current color" as "invalid color" in WebCore,
they are two functionally equivalent functions in the codebase:
- Color::isValid()
- RenderStyle::isCurrentColor(Color) which is implemented as "return !color.isValid()".

The second one makes the intention of the check clearer and should be preferred.

* Source/WebCore/animation/CSSPropertyAnimation.cpp:
* Source/WebCore/svg/properties/SVGAnimationAdditiveValueFunctionImpl.cpp:
(WebCore::SVGAnimationColorFunction::calculateDistance const):

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

87eef41

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios   πŸ›  mac βœ… πŸ›  wpe ❌ πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios   πŸ§ͺ api-mac   πŸ§ͺ api-gtk
βœ… πŸ›  tv   πŸ§ͺ mac-wk1
βœ… πŸ›  tv-sim   πŸ§ͺ mac-wk2
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch-sim   πŸ§ͺ mac-wk2-stress

@mdubet mdubet self-assigned this Sep 20, 2022
@mdubet mdubet added WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). WebKit Nightly Build labels Sep 20, 2022
@mdubet mdubet force-pushed the use-correctly-name-fn-is-currentcolor branch from 12c3b9d to 20fd0a0 Compare September 20, 2022 13:55
@nt1m
Copy link
Member

nt1m commented Sep 20, 2022

Please describe why this change is valid, so it's clear to other people reading the patch.

Also, * Source/WebCore/svg/properties/SVGAnimationAdditiveValueFunctionImpl.cpp: is missing from the commit message.

@mdubet mdubet force-pushed the use-correctly-name-fn-is-currentcolor branch from 20fd0a0 to 87eef41 Compare September 20, 2022 14:00
@mdubet mdubet changed the title Rename unclear Color::isValid to isCurrentColor(). Change unclear Color::isValid() to explicit isCurrentColor(). Sep 20, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 20, 2022
@nt1m nt1m 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 Sep 20, 2022
https://bugs.webkit.org/show_bug.cgi?id=245424

Reviewed by Antoine Quint.

Due to the representation of "current color" as "invalid color" in WebCore,
they are two functionally equivalent functions in the codebase:
- Color::isValid()
- RenderStyle::isCurrentColor(Color) which is implemented as "return !color.isValid()".

The second one makes the intention of the check clearer and should be preferred.

* Source/WebCore/animation/CSSPropertyAnimation.cpp:
* Source/WebCore/svg/properties/SVGAnimationAdditiveValueFunctionImpl.cpp:
(WebCore::SVGAnimationColorFunction::calculateDistance const):

Canonical link: https://commits.webkit.org/254683@main
@webkit-commit-queue
Copy link
Collaborator

Committed 254683@main (0b28244): https://commits.webkit.org/254683@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit 0b28244 into WebKit:main Sep 20, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
6 participants