Skip to content

[Style] Remove exposure of the underlying LengthType in Style::LengthWrappers #47129

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
merged 1 commit into from
Jun 24, 2025
Merged

[Style] Remove exposure of the underlying LengthType in Style::LengthWrappers #47129

merged 1 commit into from
Jun 24, 2025

Conversation

weinig
Copy link
Contributor

@weinig weinig commented Jun 24, 2025 •
edited by webkit-early-warning-system
Loading

5f322ca

[Style] Remove exposure of the underlying LengthType in Style::LengthWrappers
https://bugs.webkit.org/show_bug.cgi?id=294923

Reviewed by Darin Adler.

Remove the need for Style::LengthWrapperBase to expose the underlying Length's
LengthType property by removing all callers.

* Source/WebCore/rendering/AutoTableLayout.cpp:
* Source/WebCore/rendering/AutoTableLayout.h:
* Source/WebCore/rendering/RenderBlockFlow.cpp:
* Source/WebCore/rendering/RenderBox.cpp:
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
* Source/WebCore/rendering/RenderGrid.cpp:
* Source/WebCore/style/values/primitives/StyleLengthWrapper.h:

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

a038a92

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
  🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
  🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@weinig weinig self-assigned this Jun 24, 2025
@weinig weinig added the CSS Cascading Style Sheets implementation label Jun 24, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 688aca0)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ❌ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@weinig weinig marked this pull request as ready for review June 24, 2025 19:55
darinadler
darinadler approved these changes Jun 24, 2025
View reviewed changes
@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #41431 (688aca0)

❌ Found 1 failing file with 2 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@weinig weinig force-pushed the eng/Style-Remove-exposure-of-the-underlying-LengthType-in-Style-LengthWrappers branch from 688aca0 to a038a92 Compare June 24, 2025 21:12
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

EWS run on current version of this PR (hash a038a92)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
  🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
  🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@weinig weinig added the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025
@weinig
[Style] Remove exposure of the underlying LengthType in Style::Length…
5f322ca 
…Wrappers

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

Reviewed by Darin Adler.

Remove the need for Style::LengthWrapperBase to expose the underlying Length's
LengthType property by removing all callers.

* Source/WebCore/rendering/AutoTableLayout.cpp:
* Source/WebCore/rendering/AutoTableLayout.h:
* Source/WebCore/rendering/RenderBlockFlow.cpp:
* Source/WebCore/rendering/RenderBox.cpp:
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
* Source/WebCore/rendering/RenderGrid.cpp:
* Source/WebCore/style/values/primitives/StyleLengthWrapper.h:

Canonical link: https://commits.webkit.org/296584@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Style-Remove-exposure-of-the-underlying-LengthType-in-Style-LengthWrappers branch from a038a92 to 5f322ca Compare June 24, 2025 23:01
@webkit-commit-queue webkit-commit-queue merged commit 5f322ca into WebKit:main Jun 24, 2025
@webkit-commit-queue
Copy link
Collaborator

Committed 296584@main (5f322ca): https://commits.webkit.org/296584@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025
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

@alanbaradlay alanbaradlay Awaiting requested review from alanbaradlay

Assignees

@weinig weinig

Labels
CSS Cascading Style Sheets implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@weinig @webkit-early-warning-system @webkit-ews-buildbot @webkit-commit-queue @darinadler