All other viewport-related dimensions are referenced to by 'viewport',
so let's rename the member that stores the viewport size to prevent
further confusion.
`Element::ordinal_value` is called for every `li` element in
a list (ul, ol, menu).
Before:
`ordinal_value` iterates through all of the children of the list
owner. It is called once for each element: complexity $O(n^2)$.
After:
- Save the result of the first calculation in `m_ordinal_value`
then return it in subsequent calls.
- Tree modifications are intercepted and trigger invalidation
of the first node's `m_ordinal_value`:
- insert_before
- append
- remove
Results in noticeable performance improvement rendering' large
lists: from 20s to 4s for 20K elements.
Before:
`is<HTMLOLListElement>` and other similar calls in this commit
are resolved to `dynamic_cast`, which incurs runtime overhead
for resolving the type. The Performance hit becomes apparent
when rendering large lists. Callgrind analysis points to a
significant performance hit from calls to `is<...>` in
`Element::list_owner`.
Reference: Michael Gibbs and Bjarne Stroustrup (2006) Fast dynamic
casting. Softw. Pract. Exper. 2006; 36:139–156
After:
Implement inline `fast_is` virtual method that immediately
resolves the type. Results in noticeable performance improvement
2x-ish for lists with 20K entries.
Bonus: Convert starting value for LI elements to signed integer
The spec requires the start attribute and starting value to be
"integer". Firefox and Chrome support a negative start attribute.
FIXME: At the time of this PR, about 134 other objects resolve
`is<...>` to `dynamic_cast`. It may be a good idea to coordinate
similar changes to at least [some of] the ones that my have impact
on performance (maybe open a new issue?).
We were passing in byte offsets instead of UTF-16 code unit offsets,
which could lead to crashes if the offsets found exceeded the number of
code units in text fragments on the page.
Fixes#4908.
Co-authored-by: Tim Ledbetter <tim.ledbetter@ladybird.org>
Instead, collect a list of all the elements with content-visibility:auto
after layout.
This way we can skip the tree traversal when updating the rendering.
This was previously eating up ~300 µs of the 60fps frame budget on
our GitHub repo pages (and even more on large pages).
Previously we would just throw it away and construct a new (empty) one
when required. This doesn't work as any existing references to the old
instance will contain out of date information. Now we retain and update
the existing instance instead.
When marking a part of the layout tree for rebuild, if the subtree root
that we're marking has an anonymous parent, we now mark from the nearest
non-anonymous ancestor instead.
This ensures that subtrees inside anonymous wrappers don't just get
duplicated (i.e recreated but inserted instead of replaced).
For example, button inputs shouldn't have a cursor
displayed in their text since they're not editable,
and are not meant to be editable.
Fixes#4140
Co-authored-by: Sam Atkins <sam@ladybird.org>
Make copies of the animation timeline list and animations to dispatch
events at before iterating over them. This ensures that they can't be
modified during iteration.
We move m_pending_nodes_for_style_invalidation_due_to_presence_of_has to
a local variable before iterating over it. This ensures that nothing can
be added to it while iterating.
When an element's ID is removed we only want to remove it from
`m_potentially_named_elements` if it is not also considered a
potentially named element due to it's name content attribute.
Which has an optmization if both size of the string being passed
through are FlyStrings, which actually ends up being the case
in some places during selector matching comparing attribute names.
Instead of maintaining more overloads of
Infra::is_ascii_case_insensitive_match, switch
everything over to equals_ignoring_ascii_case instead.
While width and height are presentational hints on canvas, they actually
map to the CSS aspect-ratio attribute, not to CSS width and height.
For this reason, we actually need to manually mark for relayout here.
Also import a WPT test that was flaky before this change.
Returning a Vector of Slottable is not very nice here, but this
matches find_slottable (which this calls), and as far as I can
tell this is technically 'safe' at the moment in the way in
which it is / will be called.
It's also not great that like find_slottable it takes a non-const
ref, but changing that causes a bunch of other fallout.
This reworks EventHandler so text insertion, backspace, delete and
return actions are now handled by the Editing API. This was the whole
point of the execCommand spec, to provide an implementation of both
editing commands and the expected editing behavior on user input.
Responsibility of firing the `input` event is moved from EventHandler to
the Editing API, which also gets rid of duplicate events whenever
dealing with `<input>` or `<textarea>` events.
The `beforeinput` event still needs to be fired by `EventHandler`
however, since that is never fired by `execCommand()`.
Previously we only matched against the first attribute with a given
local name. What we actually want to do is look at each attribute with
that local name in turn and only return false if none of them match.
Also remove a hack for HTML elements in HTML documents, where we would
refuse to match any namespaced attributes. This doesn't seem to be
based on the spec, but we had regressions without it, until now. :^)
Gets us 21 more WPT subtest passes.
Our Ranges should maintain the invariant that their offsets are always
within range of 0..length (inclusive) of their respective containers.
Note that we cannot maintain this in AbstractRange, which is the base
for StaticRange and can still have invalid offsets.
156c1083e9 introduced a text blocks cache
for better performance when searching through text on a page, but when
we partially recreate the layout tree, this cache does not get
invalidated. We now rebuild the entire text blocks cache after a layout
update.
We were calling into `Range::set_start_or_end()` indirectly through
`::set_start()` and `::set_end()`, but that algorithm only calls for an
invocation whenever the start or end of a range needs to be set to a
boundary point. If an algorithm step calls for setting the node or
offset, we should directly modify the range.
The problem with calling into `::set_start_or_end()` is that this
algorithm potentially modifies _both_ the start and end of the range,
but algorithms trying to update a range's start or end often have
explicit steps to take both the start and end into account and end up
overcompensating for the start or end offset resulting in an invalid
range (e.g. with an end offset beyond a node's length).
This makes updating a range's start/end a bit more efficient and removes
a piece of ad-hoc code in CharacterData needed to make it work before.
The play_or_cancel_animations_after_display_property_change() helper
was being called by Node::inserted() and Node::removed_from() and then
recursing into the shadow-including subtree.
This had quadratic complexity since inserted() and removed_from() are
themselves already invoked recursively for everything in the
shadow-including subtree.
Only one caller of this API actually needed the recursive behavior,
so this patch moves that responsibility to the caller and puts the logic
in style recomputation instead.
1.02x speedup on Speedometer's TodoMVC-jQuery.
Otherwise, the arrow painted next to the <details> element does not
update.
Using a screenshot test here because apparently the direction of the
arrow has no effect on the layout or paint trees.
After f7a3f785a8, sibling nodes' styles
were no longer invalidated after a node was removed. This reuses the
flag for `:first-child` and `:last-child` to indicate that a node's
style might be affected by any structural change in its siblings.
Fixes#4631.
Resolves the `:only-child` ACID3 failure as documented in #1231.
This is the simplest fix I could find that resolves a buggy interaction
between this and the CSS fetch algorithms, which also doesn't regress
anything. (As far as I can tell.)
These are not associated with a javascript realm, so to avoid
confusion about which realm these need to be created in, make
all of these objects a GC::Cell, and deal with the fallout.
For attributes like Element.ariaControlsElements, which are a reflection
of FrozenArray<Element>, we must return the same JS::Array object every
time the attribute is invoked - until its contents have changed. This
patch implements caching of the reflected array in accordance with the
spec.
