servo / servo

Servo, the embeddable, independent, memory-safe, modular, parallel web rendering engine
https://servo.org
Mozilla Public License 2.0
28.54k stars 3.04k forks source link

Alignment should affect size of absolutely positioned element #34248

Open Loirooriol opened 1 week ago

Loirooriol commented 1 week ago
<!DOCTYPE html>
<style>
.wrapper { display: inline-block; position: relative; width: 100px; height: 100px; border: 3px solid; }
.abspos { position: absolute; inset: 0; width: auto; height: auto; background: cyan; }
.abspos::before { content: ""; display: block; width: 50px; height: 50px; }
</style>
<div class="wrapper"><div class="abspos" style="place-self: normal"></div></div>
<div class="wrapper"><div class="abspos" style="place-self: stretch"></div></div>
<div class="wrapper"><div class="abspos" style="place-self: center"></div></div>
<br>
<div class="wrapper"><canvas width="50" height="50" class="abspos" style="place-self: normal"></canvas></div>
<div class="wrapper"><canvas width="50" height="50" class="abspos" style="place-self: stretch"></canvas></div>
<div class="wrapper"><canvas width="50" height="50" class="abspos" style="place-self: center"></canvas></div>
Servo (bad) Blink (good)

The replaced and non-replaced cases can be addressed in different PRs.

The non-replaced case (1st row in testcase) seems simple, currently it always stretches because of https://github.com/servo/servo/blob/3fd1a229df65406699c5795cf504948cdb314320/components/layout_2020/positioned.rs#L849

It should use Size::FitContent when the alignment isn't normal or stretch.

taniishkaaa commented 1 week ago

i'd like to try this!

taniishkaaa commented 1 week ago

I think we can move on to the replaced case now?

Loirooriol commented 1 week ago

That needs to be handled in used_size_as_if_inline_element_from_content_box_sizes, which is not only used for abspos, but for now we should probably only do this for abspos.

So I guess it would be a matter of passing the inline_alignment and block_alignment as a LogicalVec2<AlignFlags> argument, and other callers would just use AlignFlags::START. Or maybe a auto_size_stretches_to_containing_block: LogicalVec2<bool> or initial_size_behavior: LogicalVec2<Size<Au>>?

Then these may need to use Size::Stretch:

And these may need a new parameter as suggested in https://github.com/servo/servo/blob/997b6411c034a3d8ea3d285e24cca982d4b4f3e8/components/layout_2020/geom.rs#L842

taniishkaaa commented 5 days ago

I've opened a draft PR to make sure I'm on the right track with this. Currently, all the test cases stretch. Screenshot 2024-11-21 004113