In 89ba00304c, the box' X position was
capped at 0 to prevent negative X positions to act as if there were
intruding floats on the left side. Instead, we need to check whether the
left side float intrusion we are going to calculate matters at all -
because if there's no matching float box, the intrusion is always going
to be 0 and we don't need to take the box' X position into account.
Fixes the floating publication images on https://lexfridman.com/.
This behavior is part of the cyclic percentage contribution logic from
CSS-SIZING-3 which explicitly only applies to non-replaced boxes.
This fixes an issue on Discord where buttons in the settings UI were
cropped to a narrower width than intended.
Fixes#3572
While 788d5368a7 took care of better text
marker positioning, this improves graphical marker positioning instead.
By looking at how Firefox and Chrome render markers, it's clear that
there are three parts to positioning a graphical marker:
* The containing space that the marker resides in;
* The marker dimensions;
* The distance between the marker and the start of the list item.
The space that the marker can be contained in, is the area to the left
of the list item with a height of the marker's line-height. The marker
dimensions are relative to the marker's font's pixel size: most of them
are a square at 35% of the font size, but the disclosure markers are
sized at 50% instead. Finally, the marker distance is always gauged at
50% of the font size.
So for example, a list item with `list-style-type: disc` and `font-size:
20px`, has 10px between its start and the right side of the marker, and
the marker's dimensions are 7x7.
The percentages I've chosen closely resemble how Firefox lays out its
list item markers.
This commit is a three-parter that is hard to separate without breaking
marker rendering:
1. Any marker style that results in a string, except for a literal
string (e.g. `list-style-type: "@"`), should get the string ". "
appended. We forgot to do this for the alpha and roman types.
2. Instead of using the "pixel size rounded up" from a font and adding
an arbitrary 1 to that, we now use the exact pixel size for as long
as possible to improve our vertical positioning of markers.
3. Instead of always adding a "default marker width" to the marker
content width, we now only do this if we did not have text metrics
available (i.e. the marker style is not a text type). This greatly
improves horizontal positioning of text markers.
Before this change, we were always behaving as if box-sizing were
content-box for block-level replaced element widths.
This fixes the squishy logo on https://videolan.org/
Whenever we end up with a scrollable overflow rect that goes beyond
either of its axes (i.e. the rect has a negative X or Y position
relative to its parent's absolute padding box position), we need to clip
that rect to prevent going into the "unreachable scrollable overflow".
This fixes the horizontal scrolling on https://ladybird.org (gets more
pronounced if you make the window very narrow).
...with inline children. This fixes an issue when we ignore abspos boxes
contained by PaintableWithLines while calculating overflow rect size.
Lots of layout tests are affected, because now PaintableWithLines has
overflow rect.
`Text/input/DOM/Element-set-scroll-left.html` is also affected and now
matches other browsers.
We were only dumping a PaintableBox' dimensions if its layout node was a
Layout::Box as well, causing us to not dump the dimensions of paintables
for inline nodes in the paintable tree.
In `InlineLevelIterator`, whenever we call `skip_to_next()` and enter a
node with box model metrics, we could potentially accumulate leading and
trailing metrics. This lead to a weird situation where an element with
`display: inline-block` could adopt the leading metrics of an inline
element that follows it, since we perform the call to
`add_extra_box_model_metrics_to_item()` too late.
Move `skip_to_next()` down so it no longer interferes with the `Item`
we're creating.
The test expectation for
`atomic-inline-with-percentage-vertical-align.html` is updated, although
neither the old nor new results are 100% accurate since either box jumps
one pixel to the right.
We used to subtract the maximum right margin of any containing box,
but we want to subtract the entire right margin instead. This yielded
incorrect intrusions for right floats not placed in the root.
When restructuring a block node inside an inline parent, if the
nearest block ancestor is `display: inline-block`, ensure that
the generated anonymous wrappers also have `display: inline-block`.
This fixes layout issues with block elements nested
inside inline-block elements.
Previously floats would be placed next to the highest float that
fitted the new float on its line. However, this violates the rule
that floats should be placed under the preceding float if it does
not fit next to it.
Originally part of a fix in 15103d172c, it
appears that this is no longer necessary and received a better fix in a
more recent commit. Resolves a visual regression with the ACID3 test.
We would only correct for the left margin of the containing block of the
child box that we are laying out, but we actually need to correct for
all left margins up until the containing block that contains both the
float and the child box.
Fixes#4639.
Browsers such as Chrome and Firefox apply an arbitrary scale to the
current font size if `normal` is used for `line-height`. Firefox uses
1.2 while Chrome uses 1.15. Let's go with the latter for now, it's
relatively easy to change if we ever want to go back on that decision.
This also requires updating the expectations for a lot of layout tests.
The upside of this is that it's a bit easier to compare our layout
results to other browsers', especially Chrome.
Whenever we introduce a block element in a container that at that point
has only had inline children, we create an anonymous wrapper for all the
inline elements so we can keep the invariant that each container
contains either inline or non-inline children. For some reason, we
ignore all the out-of-flow nodes since they are layed out separately and
it was thought that this shouldn't matter.
However, if we are dealing with inline blocks and floating blocks, the
order of the inline contents _including_ out-of-flow nodes becomes very
important: floating blocks need to take the order of nodes into account
when positioning themselves.
Fix this by simply hoisting the out-of-flow nodes into the anonymous
wrapper as well.
Fixes the order of blocks in #4212. The gap is still not present.
In 15103d172c we applied any remaining vertical float clearance to the
BFC's current Y offset for the next block to layout, because a `<br
style="clear: left">` could introduce clearance that would otherwise be
ignored. However, a `<div>` that floats _and_ clears `right` also
introduces this clearance and it is obvious that this should not push
down any subsequent blocks to layout in the current BFC.
Turns out, we don't need this change anymore. Some other later change
also fixed the underlying issue, and by getting rid of the original fix
we can now render https://en.wikipedia.org/wiki/SerenityOS correctly
again.
Fixes#4418.
When determining the content/margin box rects within their ancestor's
coordinate space, we were returning early if the passed in values
already belonged to the requested ancestor. Unfortunately, we had
already applied the used values' offset to the rect, which is the offset
to the ancestor's ancestor.
This simplifies the logic to always apply the rect offset after checking
if we've reached the ancestor. Fixes determining float intrusions inside
block elements with `margin: auto` set.
Fixes#4083.
If a block with inline children ends with a line break clearing any
floats, we not only need to take the introduced clearance into account
for the next line box, but the containing block needs to set the correct
height as well.
Since the spec calls for using the last line box' bottom as the resolved
height (if treated as auto), we now correctly apply the clearance to the
previous line box' bottom coordinate.
Fixes#4058.
We were accidentally providing it with absolute Y-coordinates, messing
up stacked floating boxes that would otherwise intrude on each other.
Fixes#4160.
Whenever we generate line boxes, we might end up with a residual
vertical float clearance by way of having a `<br>` with `clear: ..` set.
Set the Y offset of the next block level box to place by this vertical
clearance.
Relates to #4058.
We were introducing a line break and applying vertical clearance to the
inline formatting context, but that vertical clearance only applied to
new floating boxes. We should move the current block offset to the
vertical clearance to make sure the next line box starts beyond the
cleared floats.
There was a layout test for `<br>` with `clear: ..` set, but that test
did not actually do anything - removing the `clear` property would
result in the same layout. Replace that test with something that
actually tests float clearing.
Relates to #4058.
Our recent change to get rid of the "move 1px at a time" algorithm in
the float positioning logic introduced the issue that potentially
intersecting float boxes were not evaluated in order anymore. This could
result in float boxes being pushed down further than strictly necessary.
By finding the highest point we can move the floating box to and
repeating the process until we're no longer intersecting any floating
box, we also solve some edge cases like intersecting with very long
floating boxes whose edges lay outside the current box' edges.
This is by no means the most efficient solution, but it is more correct
than what we had until now.
Fixes#4110.
We used to always clear the side data after encountering a box with
`clear: ..`, but this is not the right thing to do if that same box also
has `float: ..` set. For example, with `clear: right` and `float: left`
it might be that the next box still wants to clear the right side, and
since the previous box is floating it did not push the next box down far
enough to justify clearing the side data at that point.
This changes the logic to only clear the float side if the clearing box
itself is not floating. We also no longer clear the opposite side after
placing a floating box; that doesn't seem to be necessary anymore.
Fixes#4102.
When generating line boxes, we place floats simultaneously with the
other items on the same line. The CSS text spec requires us to trim the
whitespace at the end of each line, but we only did so after laying out
all the line boxes.
This changes the way we calculate the current line box width for floats
by subtracting the amount of pixels that the current trailing whitespace
is using.
Fixes#4050.
All abspos boxes are expected to be blockified, so we are certain that
we can ignore non-box elements when collecting abspos nodes for layout.
Fixes a crash caused by an attempt to cast a BreakNode to a Box while
performing abspos layout.
...boxes with non-auto height.
We know for sure that by the time we layout abspos boxes, their
containing block has definite height, so it's possible to resolve
non-auto heights and mark it as definite before doing inner layout.
Big step towards having reasonable performance on
https://demo.immich.app/photos because now we avoid a bunch of work
initiated by mistakenly invoked intersection observer callbacks.
Co-Authored-By: Andreas Kling <andreas@ladybird.org>
Height resolution assumes that when available space is definite, it
matches the size of non-anonymous containing block. With this change, we
correctly maintain this assumption when box is wrapped in anonymous
node.
Fixes https://github.com/LadybirdBrowser/ladybird/issues/3422
When setting `font-family: monospace;` in CSS, we have to interpret
the keyword font sizes (small, medium, large, etc) as slightly smaller
for historical reasons. Normally the medium font size is 16px, but
for monospace it's 13px.
The way this needs to behave is extremely strange:
When encountering `font-family: monospace`, we have to go back and
replay the CSS cascade as if the medium font size had been 13px all
along. Otherwise relative values like 2em/200%/etc could have gotten
lost in the inheritance chain.
We implement this in a fairly naive way by explicitly checking for
`font-family: monospace` (note: it has to be *exactly* like that,
it can't be `font-family: monospace, Courier` or similar.)
When encountered, we simply walk the element ancestors and re-run the
cascade for the font-size property. This is clumsy and inefficient,
but it does work for the common cases.
Other browsers do more elaborate things that we should eventually care
about as well, such as user-configurable font settings, per-language
behavior, etc. For now, this is just something that allows us to handle
more WPT tests where things fall apart due to unexpected font sizes.
To learn more about the wonders of font-size, see this blog post:
https://manishearth.github.io/blog/2017/08/10/font-size-an-unexpectedly-complex-css-property/
By the time we're measuring the height of a BFC root, we've already
collapsed all relevant margins for the root and its descendants.
Given this, we should simply use 0 (relative to the BFC root) as the
lowest block axis coordinate (i.e Y value) for the margin edges.
This fixes a long-standing issue where BFC roots were sometimes not tall
enough to contain their children due to margins.
This was mainly a matter of deferring the wrapping of the button's
children until after its internal layout tree has been constructed.
That way we don't lose any pseudo elements spawned along the way.
Fixes#2397.
Fixes#2399.
When positioning floats against an edge, we are taking all current
relevant floats at that side into account to determine the Y offset at
which to place the new float. However, we were using the margin box
height instead of the absolute bottom position, which disregards the
current float's Y-position within the root, and we were setting the Y
offset to that height, instead of taking the new float's Y position
inside of the root into account.
The new code determines the lowest margin bottom value within the root
of the current floats, and adds the difference between that value and
the new float's Y position to the Y offset.
This improves the quality of our font rendering, especially when
animations are involved. Relevant changes:
* Skia fonts have their subpixel flag set, which means that individual
glyphs are rendered at subpixel offsets causing glyph runs as a
whole to look better.
* Fragment offsets are no longer rounded to whole device pixels, and
instead the floating point offset is kept. This allows us to pass
through the floating point baseline position all the way to the Skia
calls, which already expected that to be a float position.
The `scrollable-contains-table.html` ref test needed different table
headings since they would slightly inflate the column size in the test
file, but not the reference.
InlinePaintable was an ad-hoc paintable type required to support the
fragmentation of inline nodes across multiple lines. It existed because
there was no way to associate multiple paintables with a single layout
node. This resulted in a lot of duplicated code between PaintableBox and
InlinePaintable. For example, most of the CSS properties like
background, border, shadows, etc. and hit-testing are almost identical
for both of them. However, the code had to be duplicated to account for
the fact that InlinePaintable creates a box for each line. And we had
quite many places that operate on paintables with a code like:
```
if (box.is_paintable_box()) {
// do something
} else (box.is_inline_paintable()) {
// do exactly the same as for paintable box but using InlinePaintable
}
```
This change replaces the usage of `InlinePaintable` with
`PaintableWithLines` created for each line, which is now possible
because we support having multiple paintables per layout node. By doing
that, we remove lots of duplicated code and bring our implementation
closer to the spec.
It's possible to resolve box's height without doing inner layout, when
computed value is not auto. Doing that fixes height resolution, when box
with percentage height has containing block with percentage height.
Before:
- resolve used width
- layout box's content
- resolve height
After:
- resolve used width
- resolve height if treated as not auto
- layout box's content
- resolve height if treated as auto
When a block container has `clear` set and some clearance is applied,
that clearance prevents margins from adjoining and therefore resets
the margin state. But when a floating box has `clear` set, that
clearance only goes between floating boxes so should not reset margin
state. BlockFormattingContexts already do that correctly, and this PR
changes InlineFormattingContext to do the same.
Fixes#1462; adds reduced input from that issue as test.
The `calculate_inner_width()` and `calculate_inner_height()` resolve
percentage paddings using the width returned by
`containing_block_width_for()`. However, this function does not account
for grids where the containing block is defined by the grid area to
which an item belongs.
This change fixes the issue by modifying `calculate_inner_width()` and
`calculate_inner_height()` to use the already resolved paddings from the
layout state. Corresponding changes ensure that paddings are resolved
and saved in the state before box-sizing is handled.
As a side effect, this change also improves abspos layout for BFC where
now paddings are resolved using padding box of containing block instead
of content box of containing block.
Append text chunks to either the start or end of the text fragment,
depending on the text direction. The direction is determined by what
script its code points are from.
