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.

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

effective_vertical_align_for_inline_layout is bogus and needs to be removed #31634

Closed
Loirooriol opened this issue Mar 12, 2024 · 2 comments · Fixed by #31636
Closed

effective_vertical_align_for_inline_layout is bogus and needs to be removed #31634

Loirooriol opened this issue Mar 12, 2024 · 2 comments · Fixed by #31636
Assignees
@Loirooriol
Labels
A-layout/inline A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020

Comments

@Loirooriol
Copy link
Contributor 

<!DOCTYPE html>
<style>
.container {
  width: 50px;
  vertical-align: top;
  border: solid;
}
</style>
<div class="container" style="display: flow-root">
  <canvas width="50" height="50" style="background: cyan"></canvas>
</div>
<div class="container" style="display: inline-block">
  <canvas width="50" height="50" style="background: cyan"></canvas>
</div>

The 1st case is good, the 2nd is bad.

What happens is that, in the IFC established by the containers, they generate the strut using the effective_vertical_align_for_inline_layout() of the container! That just happens to work if the container is block-level because we get baseline, but if fails for inline-level containers.

We should just use baseline in all cases and get rid of effective_vertical_align_for_inline_layout.
@Loirooriol Loirooriol added A-layout/inline A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 labels Mar 12, 2024
@Loirooriol Loirooriol self-assigned this Mar 12, 2024
@mrobinson
Copy link
Member

Perhaps this should be checking if the container is the root of the inline formatting context instead of the display value. We don't think we want to apply something like vertical-align: top to children of the root of the inline formatting context, because they are "virtually" placed into anonymous boxes and vertical-align isn't inherited.

@Loirooriol
Copy link
Contributor Author

Yes that's right, these effective_vertical_align_for_inline_layout should be baseline at the root of the IFC, or the vertical-align when nested inside.

@Loirooriol Loirooriol mentioned this issue Mar 13, 2024
Closed
4 tasks
@Loirooriol Loirooriol linked a pull request Mar 13, 2024 that will close this issue
Fix vertical alignment at the root of an IFC #31615
Closed
4 tasks
@Loirooriol Loirooriol mentioned this issue Mar 13, 2024
Merged
4 tasks
@Loirooriol Loirooriol linked a pull request Mar 13, 2024 that will close this issue
Fix vertical alignment at the root of an IFC #31636
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Loirooriol Loirooriol

Labels
A-layout/inline A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020
Projects
None yet
Milestone
No milestone
2 participants
@mrobinson @Loirooriol