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

Setting -webkit-column-count to auto is the same as not setting it #6368

Merged
merged 1 commit into from Nov 12, 2022

Conversation

Ahmad-S792
Copy link
Contributor

@Ahmad-S792 Ahmad-S792 commented Nov 10, 2022

bc1d076

Setting -webkit-column-count to auto is the same as not setting it

Setting -webkit-column-count to auto is the same as not setting it
https://bugs.webkit.org/show_bug.cgi?id=247745

Reviewed by Alan Baradlay.

Merge - https://chromium.googlesource.com/chromium/blink/+/da85d1a90375fe50a92cf46f8a49571850618294

In RenderStyle, column-count is represented by two members, one integer
specifying the count, and one bool specifying whether it's auto or not. If
column-count is auto, the integer is ignored, but if the integer changes, it's
still detected as a style change (even if auto is set to true), and we'll mark
for layout for no good reason. Make sure that setting column-count to auto also
resets the integer to its initial value, to avoid this problem.

* Source/WebCore/rendering/style/RenderStyle.h: Update "setHasAutoColumnCount" to return "initialColumnCount" rather than "0"

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

67baed6

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 11, 2022
@Ahmad-S792
Copy link
Contributor Author

I have to close this bug because Webkit does not support "-webkit-columns: auto " properly and also don't have related Internals Webkit Testing Framework API to land this as it is. I have added first parsing bug as "See Also" while also explained the internal testing framework thing on the bug for future reference. Closing this now.

@Ahmad-S792 Ahmad-S792 closed this Nov 11, 2022
@alanbaradlay
Copy link
Contributor

I'd just land this patch without the test case.

@alanbaradlay alanbaradlay reopened this Nov 12, 2022
@Ahmad-S792
Copy link
Contributor Author

I'd just land this patch without the test case.

Perfect! I will update it and remove test cases and will push it for your review. Thanks!

@Ahmad-S792 Ahmad-S792 added Layout and Rendering For bugs with layout and rendering of Web pages. WebKit Nightly Build and removed merging-blocked Applied to prevent a change from being merged labels Nov 12, 2022
@Ahmad-S792 Ahmad-S792 self-assigned this Nov 12, 2022
@Ahmad-S792 Ahmad-S792 marked this pull request as ready for review November 12, 2022 14:28
@Ahmad-S792
Copy link
Contributor Author

@alanbaradlay - Pushed new build without test cases. Appreciate if you can review. Thanks!

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 12, 2022
@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 Nov 12, 2022
@Ahmad-S792
Copy link
Contributor Author

The test failure is related to Service Worker and with same patch previous run was fine and it didn't had any such failure, so IMO something in the tree failed leading to these issues. Further, it passed all tests except the introduced one, so I am going to merge this.

Setting -webkit-column-count to auto is the same as not setting it
https://bugs.webkit.org/show_bug.cgi?id=247745

Reviewed by Alan Baradlay.

Merge - https://chromium.googlesource.com/chromium/blink/+/da85d1a90375fe50a92cf46f8a49571850618294

In RenderStyle, column-count is represented by two members, one integer
specifying the count, and one bool specifying whether it's auto or not. If
column-count is auto, the integer is ignored, but if the integer changes, it's
still detected as a style change (even if auto is set to true), and we'll mark
for layout for no good reason. Make sure that setting column-count to auto also
resets the integer to its initial value, to avoid this problem.

* Source/WebCore/rendering/style/RenderStyle.h: Update "setHasAutoColumnCount" to return "initialColumnCount" rather than "0"

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

Committed 256607@main (bc1d076): https://commits.webkit.org/256607@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 12, 2022
@Ahmad-S792 Ahmad-S792 deleted the fix247745 branch November 14, 2022 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
5 participants