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

Do not use the CB writing mode when resolving box properties #29651

Conversation

mrobinson
Copy link
Member

Layout 2020 does a lot of work to pass the containing block writing mode around in order to try to resolve logical box properties. This seems incorrect though, as logical box model properties use the element's writing mode to resolve to physical properties [1] [2]. This is handled correctly by the style component, but Layout 2020 does a lot of work to try to override them.

This change removes that code and also tries to use more style component data structures in place of Layout 2020's custom logical geometry primitives. In particular it removes the confusingly overloaded type aliases LengthOrAuto and LengthPercentageOrAuto from Layout 2020. These aliases were very similar to the ones from the style component, but had a different definition.

Since Layout 2020 does not have proper support for writing modes, this change should not modify behavior.

  1. [css-logical] Should the mapping for logical values depend on the element or containing block? w3c/csswg-drafts#3013
  2. https://drafts.csswg.org/css-logical/#box

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because they do not change behavior.

Layout 2020 does a lot of work to pass the containing block writing mode
around in order to try to resolve logical box properties. This seems
incorrect though, as logical box model properties use the element's
writing mode to resolve to physical properties [1] [2]. This is
handled correctly by the style component, but Layout 2020 does a lot of
work to try to override them.

This change removes that code and also tries to use more style component
data structures in place of Layout 2020's custom logical geometry
primitives. In particular it removes the confusingly overloaded type
aliases `LengthOrAuto` and `LengthPercentageOrAuto` from Layout 2020.
These aliases were very similar to the ones from the style component, but
had a different definition.

Since Layout 2020 does not have proper support for writing modes, this
change should not modify behavior.

1. w3c/csswg-drafts#3013
2. https://drafts.csswg.org/css-logical/#box
@mrobinson mrobinson added the A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 label Apr 20, 2023
@mrobinson
Copy link
Member Author

After discussing with @Loirooriol, it seems that I was mistaken here. While the logical values should be resolved based on their element's writing mode, these physical values need to them be converted back to inline and block directions in the coordinate space of the containing block. There's some work from this PR that can be salvaged, particularly removing the duped LengthOrAuto and LengthPercentageOrAuto overloads and using more logical geometry types from the style system, but we can handle those little bit little in future PRs.

@mrobinson mrobinson closed this Apr 21, 2023
@mrobinson mrobinson deleted the do-not-use-cb-writing-mode-for-box-properties branch June 1, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant